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