[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range
Alex Lorenz via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 10 09:57:32 PST 2017
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;
> 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.
More information about the cfe-commits