ojhunt wrote:

> > > So I take it we decided not to enable it by default in 
> > > `-fms-compatibility` mode then?
> > 
> > 
> > I don't believe it is appropriate to do so. The intent of this warning is 
> > to indicate MSVC compatibility issues when building in non-ms-compatibility 
> > modes. The existing padding warnings already trigger for these layouts in 
> > the relevant ms compatibility modes
> 
> We don't typically add new off-by-default warnings though; users don't enable 
> them often enough to be worth the costs. My thought process is: if we enable 
> this diagnostic by default in `-fms-compatibility` mode as telling the user 
> their bit-fields aren't packing, then it's on by default for some 
> configurations so it meets our bar for inclusion, and it helps us directly 
> because we have Windows precommit (and post-commit) CI coverage for building 
> Clang and LLVM. Users on other platforms can opt-in to the diagnostic if they 
> want the diagnostics for compatibility reasons.

My concerns with attaching it to `-fms-compatibility` is that projects using 
that and that care about padding already have the padding/packing warnings 
enabled. For every other user we will be warning them that the padding matches 
the abi they're targeting. The problem I'm wanting to address is not "I am 
targeting the ms abi" it is "I am not targeting ms abi but want to be told 
about packing/padding that would be different on ms platforms".

The intention would be to then enable the warning (maybe with a `-Werror=...` 
once the build is clean :D ) for all llvm projects, not just the windows 
targets - maybe with some evangelism so larger projects that have similar 
issues can be made aware of the flag. Then we can start migrating away from 
unending unsigned bit-fields and adopt enum bit-fields

> 
> If we want to leave it off by default, then I wonder if we want to roll the 
> functionality into `-Wpadded-bitfield` which already exists and is off by 
> default.

The problem with this is that that people who do not care about windows/ms abi 
will then be getting what to them are spurious warnings.

https://github.com/llvm/llvm-project/pull/117428
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to