xbolva00 added a comment. In D63082#1564114 <https://reviews.llvm.org/D63082#1564114>, @aaron.ballman wrote:
> In D63082#1560667 <https://reviews.llvm.org/D63082#1560667>, @xbolva00 wrote: > > > I wanted to follow GCC’s behaviour -Wall but then dicussion moved to > > “tautological compare group” - I was fine with that too.. > > I can again revert it to -Wall... > > If this should be off by default even on -Wall, then all work was useless.. > > > Personally, my feeling is that this new diagnostic group belongs under the > tautological compare group. That group is currently off by default, but I'm > wondering if we want it to be on by default (in a subsequent patch), or > whether we're okay having parts of it on by default and parts off by default. > I'm hoping @rsmith will voice an opinion here. > > In D63082#1560684 <https://reviews.llvm.org/D63082#1560684>, @xbolva00 wrote: > > > “Perhaps appending something about the resulting value always being > > true|false would help most of these diagnostics be more obvious” > > > > I dont know, this could only diagnose constant multiplications (maybe this > > is already handled by some “always true” check). Anyway, I have no > > motivation to diagnose constant muls, sorry. My goal is to make this > > warning atleast as good as GCC’s implementation. > > > > Okay, we could make it better and append “; did you mean “index * 3 != 0”? > > > I don't think that actually improves anything. I do not think we want a "did > you mean" in this case because I'm not confident anyone can guess what a user > was trying for in this situation. However, telling the user the result of the > computation does give them very useful information -- it tells them the > result will always be tautologically true or false (which is what the other > tautological warnings do, as well). As it stands, the current diagnostic > wording doesn't help the user understand what's wrong with their code. I'd > like to correct that deficiency before we commit this. Thanks I decided to implement your suggestions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
