Higuoxing updated this revision to Diff 150465.
Higuoxing added a comment.
I step out a little further... I made an attempt to add a new warning
`[-Wlogical-op-parentheses-in-macros]`. Comparing to the previous one, which do
you prefer? I am new to llvm community, and as you see, this is my first patch
... so, forgive me if I made something wrong : ) suggestions are welcoming!
PS. I see something like `(x | y & z)` checking disabled in macros as well, how
to deal with this?
thanks
https://reviews.llvm.org/D47687
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
===================================================================
--- test/Sema/parentheses.c
+++ test/Sema/parentheses.c
@@ -14,6 +14,11 @@
if ((i = 4)) {}
}
+#define macro_parentheses_check(x) \
+ ( \
+ ( (void)(x) ) \
+ )
+
void bitwise_rel(unsigned i) {
(void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \
// expected-note{{place parentheses around the '==' expression to silence this warning}} \
@@ -96,6 +101,21 @@
(void)(i && i || 0); // no warning.
(void)(0 || i && i); // no warning.
+
+ macro_parentheses_check(i || i && i); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+
+ macro_parentheses_check(i || i && "I love LLVM"); // no warning.
+ macro_parentheses_check("I love LLVM" && i || i); // no warning.
+
+ macro_parentheses_check(i || i && "I love LLVM" || i); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+
+ macro_parentheses_check(i || "I love LLVM" && i || i); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+
+ macro_parentheses_check(i && i || 0); // no warning.
+ macro_parentheses_check(0 || i && i); // no warning.
}
_Bool someConditionFunc();
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12172,8 +12172,16 @@
EmitDiagnosticForLogicalAndInLogicalOr(Sema &Self, SourceLocation OpLoc,
BinaryOperator *Bop) {
assert(Bop->getOpcode() == BO_LAnd);
- Self.Diag(Bop->getOperatorLoc(), diag::warn_logical_and_in_logical_or)
+ 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)
+ << Bop->getSourceRange() << OpLoc;
+ }
+
SuggestParentheses(Self, Bop->getOperatorLoc(),
Self.PDiag(diag::note_precedence_silence)
<< Bop->getOpcodeStr(),
@@ -12205,6 +12213,7 @@
if (EvaluatesAsFalse(S, RHSExpr))
return;
// If it's "1 && a || b" don't warn since the precedence doesn't matter.
+ // And 'assert("some message" && a || b)' don't warn as well.
if (!EvaluatesAsTrue(S, Bop->getLHS()))
return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
} else if (Bop->getOpcode() == BO_LOr) {
@@ -12227,6 +12236,7 @@
if (EvaluatesAsFalse(S, LHSExpr))
return;
// If it's "a || b && 1" don't warn since the precedence doesn't matter.
+ // And 'assert(a || b && "some message")' don't warn as well.
if (!EvaluatesAsTrue(S, Bop->getRHS()))
return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
}
@@ -12309,8 +12319,11 @@
}
// 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. */) {
+ // Here we will not skip 'logical and in logical or' checking
+ // in macros, since 'assert(a || b && "some message")' is equal
+ // to '(a || b && 1)' and 'assert("some message" && a || b)' is
+ // equal to '(1 && a || b)'.
+ if (Opc == BO_LOr) {
DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5483,6 +5483,9 @@
def warn_logical_and_in_logical_or : Warning<
"'&&' within '||'">, InGroup<LogicalOpParentheses>;
+def warn_logical_and_in_logical_or_in_macros: Warning<
+ "'&&' within '||'">, InGroup<LogicalOpParenthesesInMacros>;
+
def warn_overloaded_shift_in_comparison :Warning<
"overloaded operator %select{>>|<<}0 has higher precedence than "
"comparison operator">,
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -261,6 +261,7 @@
def GlobalConstructors : DiagGroup<"global-constructors">;
def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">;
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">;
def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">;
@@ -668,6 +669,7 @@
def ParenthesesOnEquality : DiagGroup<"parentheses-equality">;
def Parentheses : DiagGroup<"parentheses",
[LogicalOpParentheses,
+ LogicalOpParenthesesInMacros,
LogicalNotParentheses,
BitwiseOpParentheses,
ShiftOpParentheses,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
