[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]

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