Higuoxing updated this revision to Diff 166583.
Higuoxing added a comment.
I update the *SuggestParentheses* function, now, it could deal with parentheses
inside macros
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
===================================================================
--- test/Sema/parentheses.c
+++ test/Sema/parentheses.c
@@ -14,7 +14,11 @@
if ((i = 4)) {}
}
-void bitwise_rel(unsigned i) {
+#define VOID_CAST(cond) ( (void)(cond) )
+#define APPLY_OPS(op0, op1, x, y, z) ( VOID_CAST(x op0 y op1 z) )
+#define APPLY_OPS_DIRECTLY(op0, op1, x, y, z) ( (void)(x op0 y op1 z) )
+
+void bitwise_rel(unsigned i, unsigned iiii) {
(void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \
// expected-note{{place parentheses around the '==' expression to silence this warning}} \
// expected-note{{place parentheses around the & expression to evaluate it first}}
@@ -96,6 +100,31 @@
(void)(i && i || 0); // no warning.
(void)(0 || i && i); // no warning.
+
+ VOID_CAST(i && i || i); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i); // no warning.
+ APPLY_OPS(&&, ||, i, i, i); // no warning.
+
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i || i && i); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:41-[[@LINE-2]]:41}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:47-[[@LINE-3]]:47}:")"
+
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i || i && iiii); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:41-[[@LINE-2]]:41}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:50-[[@LINE-3]]:50}:")"
+
+ APPLY_OPS(&&, ||, i, i, i || i && i); // no warning.
+
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i && i); // no warning.
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i || i); // no warning.
+
+ APPLY_OPS(&&, ||, i, i, i && i); // no warning.
+ APPLY_OPS(&&, ||, i, i, i || i); // no warning.
+
}
_Bool someConditionFunc();
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -7103,9 +7103,11 @@
static void SuggestParentheses(Sema &Self, SourceLocation Loc,
const PartialDiagnostic &Note,
SourceRange ParenRange) {
- SourceLocation EndLoc = Self.getLocForEndOfToken(ParenRange.getEnd());
- if (ParenRange.getBegin().isFileID() && ParenRange.getEnd().isFileID() &&
- EndLoc.isValid()) {
+ SourceManager &SM = Self.getSourceManager();
+ SourceLocation spellLoc = SM.getSpellingLoc(ParenRange.getEnd());
+ unsigned tokLen = Lexer::MeasureTokenLength(spellLoc, SM, Self.getLangOpts());
+ SourceLocation EndLoc = spellLoc.getLocWithOffset(tokLen);
+ if (EndLoc.isValid()) {
Self.Diag(Loc, Note)
<< FixItHint::CreateInsertion(ParenRange.getBegin(), "(")
<< FixItHint::CreateInsertion(EndLoc, ")");
@@ -12331,6 +12333,34 @@
}
}
+static void DiagnoseLogicalAndInLogicalOr(Sema &Self, SourceLocation OpLoc,
+ Expr *LHSExpr, Expr *RHSExpr) {
+ SourceManager &SM = Self.getSourceManager();
+ if (!OpLoc.isMacroID()) {
+ DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
+ DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
+ return;
+ }
+
+ // Diagnose parentheses, if and only if operator and its LHS, RHS
+ // are from the same argument position of first level macros
+ SourceLocation OpExpansionLoc;
+ if (!SM.isMacroArgExpansion(OpLoc, &OpExpansionLoc) ||
+ SM.getImmediateMacroCallerLoc(OpLoc).isMacroID())
+ return;
+
+ SourceLocation ExprExpansionLoc;
+ if (SM.isMacroArgExpansion(LHSExpr->getExprLoc(), &ExprExpansionLoc) &&
+ OpExpansionLoc == ExprExpansionLoc &&
+ !SM.getImmediateMacroCallerLoc(LHSExpr->getExprLoc()).isMacroID())
+ DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
+
+ if (SM.isMacroArgExpansion(RHSExpr->getExprLoc(), &ExprExpansionLoc) &&
+ OpExpansionLoc == ExprExpansionLoc &&
+ !SM.getImmediateMacroCallerLoc(RHSExpr->getExprLoc()).isMacroID())
+ DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
+}
+
/// Look for bitwise op in the left or right hand of a bitwise op with
/// lower precedence and emit a diagnostic together with a fixit hint that wraps
/// the '&' expression in parentheses.
@@ -12407,10 +12437,8 @@
// Warn about arg1 || arg2 && arg3, as GCC 4.3+ does.
// We don't warn for 'assert(a || b && "bad")' since this is safe.
- if (Opc == BO_LOr && !OpLoc.isMacroID()/* Don't warn in macros. */) {
- DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
- DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
- }
+ if (Opc == BO_LOr)
+ DiagnoseLogicalAndInLogicalOr(Self, OpLoc, LHSExpr, RHSExpr);
if ((Opc == BO_Shl && LHSExpr->getType()->isIntegralType(Self.getASTContext()))
|| Opc == BO_Shr) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
