[PATCH] D47687: fix: [Bug 18971] - Missing -Wparentheses warning

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

+ there will need to be a new standalone test for `-Wlogical-op-parentheses-in-macros`,
what it does, how it relates to `-Wlogical-op-parentheses`,`-Wparentheses`, etc.



================
Comment at: include/clang/Basic/DiagnosticGroups.td:264-265
 def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">;
+def LogicalOpParenthesesInMacros: DiagGroup<"logical-op-parentheses-in-macros">;
 def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">;
 def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
----------------
`LogicalOpParenthesesInMacros` should be in `LogicalNotParentheses` group.


================
Comment at: include/clang/Basic/DiagnosticGroups.td:672
                             [LogicalOpParentheses,
+                             LogicalOpParenthesesInMacros,
                              LogicalNotParentheses,
----------------
No need


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5486-5488
+def warn_logical_and_in_logical_or_in_macros: Warning<
+  "'&&' within '||'">, InGroup<LogicalOpParenthesesInMacros>;
+
----------------
This should be
```
def warn_logical_and_in_logical_or_in_macros :
  Warning<warn_logical_and_in_logical_or.Text>,
  InGroup<LogicalOpParenthesesInMacros>, DefaultIgnore;
```
`DefaultIgnore` is my guess.


================
Comment at: lib/Sema/SemaExpr.cpp:12175-12183
+  if (!OpLoc.isMacroID()) {
+    // if this warning is in a normal context
+    Self.Diag(Bop->getOperatorLoc(), diag::warn_logical_and_in_logical_or)
       << Bop->getSourceRange() << OpLoc;
+  } else {
+    // else this warning is in a macro context
+    Self.Diag(Bop->getOperatorLoc(), diag::warn_logical_and_in_logical_or_in_macros)
----------------
Might be simpler to do
```
Self.Diag(Bop->getOperatorLoc(), OpLoc.isMacroID() ? diag::warn_logical_and_in_logical_or_in_macros :
                                                     diag::warn_logical_and_in_logical_or)
  << Bop->getSourceRange() << OpLoc;
```


https://reviews.llvm.org/D47687