rjmccall added a comment. In D72231#1881797 <https://reviews.llvm.org/D72231#1881797>, @nickdesaulniers wrote:
> In D72231#1881784 <https://reviews.llvm.org/D72231#1881784>, @rjmccall wrote: > > > In D72231#1881760 <https://reviews.llvm.org/D72231#1881760>, > > @nickdesaulniers wrote: > > > > > In D72231#1879347 <https://reviews.llvm.org/D72231#1879347>, @rjmccall > > > wrote: > > > > > > > In D72231#1878528 <https://reviews.llvm.org/D72231#1878528>, > > > > @nathanchance wrote: > > > > > > > > > There appear to a be semantic difference between GCC and clang with > > > > > the current version of this patch which results in a lot of > > > > > additional warnings in the Linux kernel: https://godbolt.org/z/eHFJd8 > > > > > > > > > > > > Warning about casting to an enum seems clearly correct and in scope for > > > > this warning. Warning about casting to `_Bool` seems clearly incorrect > > > > and should not be warned about at all. > > > > > > > > > Maybe we should only warn if the size of the `void*` is smaller than the > > > size of the `enum`? (32b `void*`, 64b `enum`)? > > > https://godbolt.org/z/oAts-u > > > > > > Otherwise this warning creates a massive mess for us to clean up, and I > > > suspect Linux kernel developers will just end up disabling the warning. > > > > > > If deployment is easier if we split out a subgroup that we can turn off, > > that seems fine. But I don't see any good abstract justification for > > warning about a cast to `int` and not a cast to an `int`-sized `enum`. > > What would the reasoning be, just that the latter "couldn't possibly" be > > intended to preserve the original pointer value, so it must be an opaque > > value being represented as a `void*`? That seems pretty weak to me. > > > Less about enums, more about casts to/from void*, since you might use that in > place of a union that would be too large to describe. Specifically, this > `struct` is used throughout the kernel for most drivers: > https://elixir.bootlin.com/linux/v5.5.4/source/include/linux/mod_devicetable.h#L260 > It is exceedingly common to stuff whatever data in there: > https://elixir.bootlin.com/linux/v5.5.4/source/drivers/ata/ahci_brcm.c#L428 > so long as the driver is careful not to reinterpret the data as the incorrect > type. Describing such a union for ever possible enum packed in there would > not be fun. No, I understand the pattern, but they must have already done some sort of pass over the code to make it warning-clean when they're working with a smaller integer type. Or do they just in practice never store smaller integers in there, whereas it's hard to control size with an enum? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72231/new/ https://reviews.llvm.org/D72231 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
