aaron.ballman added inline comments.

================
Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92
+  if (N.getNodeAs<NamedDecl>("std_type"))
+    diag(Location, "shifting a value of the standardized bitmask types");
+  else
----------------
JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > How about: "shifting a value of bitmask type"
> > > Not sure about that.
> > > 
> > > The general bitmasks are covered by the second `diag`. 
> > > 
> > > This one should only trigger if a shift with the standardized bitmask 
> > > types occurs. This exception is necessary because those are allowed for 
> > > the other bitwise operations(&, |, ^), but i decided that shifting them 
> > > makes no sense. 
> > I think "standardized bitmask type" is not very clear because it's hard to 
> > know what is and isn't a standardized bitmask type (it's not a term of art 
> > I'm used to hearing, anyway). I agree that shifting by them makes no sense, 
> > but I also have a hard time imagining anyone is using these as shift values 
> > in the first place, so perhaps the diagnostic can be removed entirely 
> > unless we can find some code in the wild that does something like this?
> I totally agree that its bad.
> 
> Removing this exception is ok with me.
Okay, we'll go that route -- we can always bring it back if it turns out to 
have value later. Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45414



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to