[PATCH] D91651: [clang] Add a warning (à la gcc) for too small enum bitfields

Faisal Vali via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 25 21:34:39 PST 2020


faisalv added a comment.

In D91651#2416423 <https://reviews.llvm.org/D91651#2416423>, @thakis wrote:

> Do you have any numbers on false positives / true positives uncovered by this tweak?

That's a great question - and unfortunately not only do I have no hard data to support or discourage the addition of such a warning - I don't even know how some of those incredible folks (who make data based claims about programming smells and needs) gather such data (leave alone making sure the data is a representative sample for the "right" kind of programmers) - and would love to be shown how to run such studies :)

> In general, warning at use time instead of at declaration time tends to be much better for this rate, and we do this differently than gcc in several instances, because the gcc way has so many false positives that it makes warnings unusable, while warning on use makes the warning useful. So I'd like to see a better justification than "gcc does it" :)

Aah - i did not realize that it was a deliberate decision not to implement such a warning at definition time.  My justification (asides from gcc being the light for us in dark places, when all other lights go out ;) for implementing a warning at definition time probably just stems from an instinctual preference for early diagnostics - i suppose there is no one size that fits all here - for e.g. some folks prefer run-time (duck-typing) operation checking vs compile-time checking, and others who prefer diagnosing fundamental issues with template-code when they are first parsed, as opposed to waiting for some instantiation - and then there is the entire C++0X concepts vs concepts-lite discussion ...

So, since I feel I lack the authority to justify such a warning (asides from my ideological propensities) in the face of your valid concerns (either anecdotal or fueled by sufficient data) - I would prefer to defer to you and withdraw the patch :)  (unless someone else feels they are able to provide justification that might resonate with you).

Thanks for chiming in!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91651/new/

https://reviews.llvm.org/D91651



More information about the cfe-commits mailing list