[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 10 10:22:50 PDT 2018


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





More information about the cfe-commits mailing list