arphaman added inline comments.
================ Comment at: lib/Analysis/ReachableCode.cpp:229 + if (SilenceableCondValNotSet && SilenceableCondVal->getBegin().isValid()) + *SilenceableCondVal = UO->getSourceRange(); + return UO->getOpcode() == UO_LNot && IsSubExprConfigValue; ---------------- ahatanak wrote: > Thanks, that fixed the incorrect fixit. The patch looks good to me, but there > are still cases in which the suggestions clang makes are not accurate. > Perhaps you can leave a FIXME somewhere so that we don't forget to fix it > later? > > For example, if we change the condition in the test case to this, > > ``` > if (!(s->p && 0)) > ``` > > , clang suggests enclosing the entire expression with a parenthesis (caret > points to "!"), but the warning will not go away unless the parenthesis is > around "0". > > The second example is in function unaryOpFixit in warn-unreachable.c. clang > suggests enclosing "-1" with a parenthesis, but it still warns after putting > the parenthesis around "-1" or "1". Thanks for pointing out these issues, I've decided to fix them in this patch. Since only the '!' operator can wrap a silenced expression, the updated patch ensures that fixits for unary operators can only be used with '!' (this fixes your second example). I fixed the first example by ensuring that the unary operator source range can be used for fixit only if the previous fixit source range comes from its direct child. Repository: rL LLVM https://reviews.llvm.org/D28231 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
