nickdesaulniers added a comment.

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.


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

Reply via email to