On Apr 10, 2013 10:04 AM, "Richard Trieu" <[email protected]> wrote: > > Streams commonly overload the << and >> operators for input and output. However, these operators have higher precedence than comparison operators. Code written like this: > > cout << 5 == 4; > > Is interpreted as: > > (cout << 5) == 4; > > Instead of: > > cout << (5 == 4);
What sort of statistics on success rate do you have with this warning? I wonder if it would find many intentional uses when dealing with numeric user defined tyows,. > > http://llvm-reviews.chandlerc.com/D650 > > Files: > lib/Sema/SemaExpr.cpp > test/Sema/parentheses.cpp > include/clang/Basic/DiagnosticSemaKinds.td > include/clang/Basic/DiagnosticGroups.td > > Index: lib/Sema/SemaExpr.cpp > =================================================================== > --- lib/Sema/SemaExpr.cpp > +++ lib/Sema/SemaExpr.cpp > @@ -8819,6 +8819,27 @@ > } > } > > +static void DiagnoseShiftCompare(Sema &S, SourceLocation OpLoc, > + Expr *LHSExpr, Expr *RHSExpr) { > + CXXOperatorCallExpr *OCE = dyn_cast<CXXOperatorCallExpr>(LHSExpr); > + if (!OCE) return; These early returns seem a little hard to spot in a fairly dense/flat piece if code. Might benefit from putting the return on their own line and/or some comments (or perhaps this is easy enough to read with syntax highlighting) > + FunctionDecl *FD = OCE->getDirectCallee(); > + if (!FD || !FD->isOverloadedOperator()) return; > + OverloadedOperatorKind Kind = FD->getOverloadedOperator(); > + if (Kind != OO_LessLess && Kind != OO_GreaterGreater) return; > + S.Diag(OpLoc, diag::warn_overloaded_shift_in_comparison) > + << LHSExpr->getSourceRange() << RHSExpr->getSourceRange() > + << (Kind == OO_LessLess); > + SuggestParentheses(S, OpLoc, > + S.PDiag(diag::note_evaluate_comparison_first), > + SourceRange(OCE->getArg(1)->getLocStart(), > + RHSExpr->getLocEnd())); > + SuggestParentheses(S, OCE->getOperatorLoc(), > + S.PDiag(diag::note_precedence_silence) > + << (Kind == OO_LessLess ? "<<" : ">>"), > + OCE->getSourceRange()); > +} > + > /// DiagnoseBinOpPrecedence - Emit warnings for expressions with tricky > /// precedence. > static void DiagnoseBinOpPrecedence(Sema &Self, BinaryOperatorKind Opc, > @@ -8847,6 +8868,11 @@ > DiagnoseAdditionInShift(Self, OpLoc, LHSExpr, Shift); > DiagnoseAdditionInShift(Self, OpLoc, RHSExpr, Shift); > } > + > + // Warn on overloaded shift operators and comparisons, such as: > + // cout << 5 == 4; > + if (BinaryOperator::isComparisonOp(Opc)) > + DiagnoseShiftCompare(Self, OpLoc, LHSExpr, RHSExpr); > } > > // Binary Operators. 'Tok' is the token for the operator. > Index: test/Sema/parentheses.cpp > =================================================================== > --- test/Sema/parentheses.cpp > +++ test/Sema/parentheses.cpp > @@ -29,6 +29,12 @@ > (void)(s << b ? "foo" : "bar"); // expected-warning {{operator '?:' has lower precedence than '<<'}} \ > // expected-note {{place parentheses around the '?:' expression to evaluate it first}} \ > // expected-note {{place parentheses around the '<<' expression to silence this warning}} > + (void)(s << 5 == 1); // expected-warning {{overloaded operator << has lower precedence than comparison operator}} \ > + // expected-note {{place parentheses around the '<<' expression to silence this warning}} \ > + // expected-note {{place parentheses around comparison expression to evaluate it first}} > + (void)(s >> 5 == 1); // expected-warning {{overloaded operator >> has lower precedence than comparison operator}} \ > + // expected-note {{place parentheses around the '>>' expression to silence this warning}} \ > + // expected-note {{place parentheses around comparison expression to evaluate it first}} > } > > struct S { > Index: include/clang/Basic/DiagnosticSemaKinds.td > =================================================================== > --- include/clang/Basic/DiagnosticSemaKinds.td > +++ include/clang/Basic/DiagnosticSemaKinds.td > @@ -4025,6 +4025,13 @@ > def warn_logical_and_in_logical_or : Warning< > "'&&' within '||'">, InGroup<LogicalOpParentheses>; > > +def warn_overloaded_shift_in_comparison :Warning< > + "overloaded operator %select{>>|<<}0 has lower precedence than " > + "comparison operator">, > + InGroup<OverloadedShiftOpParentheses>; > +def note_evaluate_comparison_first :Note< > + "place parentheses around comparison expression to evaluate it first">; > + > def warn_addition_in_bitshift : Warning< > "operator '%0' has lower precedence than '%1'; " > "'%1' will be evaluated first">, InGroup<ShiftOpParentheses>; > Index: include/clang/Basic/DiagnosticGroups.td > =================================================================== > --- include/clang/Basic/DiagnosticGroups.td > +++ include/clang/Basic/DiagnosticGroups.td > @@ -122,6 +122,7 @@ > def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">; > def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">; > def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">; > +def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">; > def DanglingElse: DiagGroup<"dangling-else">; > def DanglingField : DiagGroup<"dangling-field">; > def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">; > @@ -352,6 +353,7 @@ > [LogicalOpParentheses, > BitwiseOpParentheses, > ShiftOpParentheses, > + OverloadedShiftOpParentheses, > ParenthesesOnEquality, > DanglingElse]>; > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
