diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -13031,6 +13031,130 @@ }); } + void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *CXXOCE) { + // C++17 [over.match.oper]p2: + // [...] the operator notation is first transformed to the equivalent + // function-call notation as summarized in Table 12 (where @ denotes one + // of the operators covered in the specified subclause). However, the + // operands are sequenced in the order prescribed for the built-in + // operator (Clause 8). + // + // From the above only overloaded binary operators and overloaded call + // operators have sequencing rules in C++17 that we need to handle + // separately. + if (!SemaRef.getLangOpts().CPlusPlus17 || + (CXXOCE->getNumArgs() != 2 && CXXOCE->getOperator() != OO_Call)) + return VisitCallExpr(CXXOCE); + + enum { + NoSequencing, + LHSBeforeRHS, + RHSBeforeLHS, + LHSBeforeRest + } SequencingKind; + switch (CXXOCE->getOperator()) { + case OO_Equal: + case OO_PlusEqual: + case OO_MinusEqual: + case OO_StarEqual: + case OO_SlashEqual: + case OO_PercentEqual: + case OO_CaretEqual: + case OO_AmpEqual: + case OO_PipeEqual: + case OO_LessLessEqual: + case OO_GreaterGreaterEqual: + SequencingKind = RHSBeforeLHS; + break; + + case OO_LessLess: + case OO_GreaterGreater: + case OO_AmpAmp: + case OO_PipePipe: + case OO_Comma: + case OO_ArrowStar: + case OO_Subscript: + SequencingKind = LHSBeforeRHS; + break; + + case OO_Call: + SequencingKind = LHSBeforeRest; + break; + + default: + SequencingKind = NoSequencing; + break; + } + + if (SequencingKind == NoSequencing) + return VisitCallExpr(CXXOCE); + + // This is a call, so all subexpressions are sequenced before the result. + SequencedSubexpression Sequenced(*this); + + SemaRef.runWithSufficientStackSpace(CXXOCE->getExprLoc(), [&] { + assert(SemaRef.getLangOpts().CPlusPlus17 && + "Should only get there with C++17 and above!"); + assert((CXXOCE->getNumArgs() == 2 || CXXOCE->getOperator() == OO_Call) && + "Should only get there with an overloaded binary operator" + " or an overloaded call operator!"); + + if (SequencingKind == LHSBeforeRest) { + assert(CXXOCE->getOperator() == OO_Call && + "We should only have an overloaded call operator here!"); + + // This is very similar to VisitCallExpr, except that we only have the + // C++17 case. The postfix-expression is the first argument of the + // CXXOperatorCallExpr. The expressions in the expression-list, if any, + // are in the following arguments. + // + // Note that we intentionally do not visit the callee expression since + // it is just a decayed reference to a function. + SequenceTree::Seq PostfixExprRegion = Tree.allocate(Region); + SequenceTree::Seq ArgsRegion = Tree.allocate(Region); + SequenceTree::Seq OldRegion = Region; + + assert(CXXOCE->getNumArgs() >= 1 && + "An overloaded call operator must have at least one argument" + " for the postfix-expression!"); + const Expr *PostfixExpr = CXXOCE->getArgs()[0]; + llvm::ArrayRef Args(CXXOCE->getArgs() + 1, + CXXOCE->getNumArgs() - 1); + + // Visit the postfix-expression first. + { + Region = PostfixExprRegion; + SequencedSubexpression Sequenced(*this); + Visit(PostfixExpr); + } + + // Then visit the argument expressions. + Region = ArgsRegion; + for (const Expr *Arg : Args) + Visit(Arg); + + Region = OldRegion; + Tree.merge(PostfixExprRegion); + Tree.merge(ArgsRegion); + } else { + assert(CXXOCE->getNumArgs() == 2 && + "Should only have two arguments here!"); + assert((SequencingKind == LHSBeforeRHS || + SequencingKind == RHSBeforeLHS) && + "Unexpected sequencing kind!"); + + // We do not visit the callee expression since it is just a decayed + // reference to a function. + const Expr *E1 = CXXOCE->getArg(0); + const Expr *E2 = CXXOCE->getArg(1); + if (SequencingKind == RHSBeforeLHS) + std::swap(E1, E2); + + return VisitSequencedExpressions(E1, E2); + } + }); + } + void VisitCXXConstructExpr(const CXXConstructExpr *CCE) { // This is a call, so all subexpressions are sequenced before the result. SequencedSubexpression Sequenced(*this); diff --git a/clang/test/SemaCXX/warn-unsequenced.cpp b/clang/test/SemaCXX/warn-unsequenced.cpp --- a/clang/test/SemaCXX/warn-unsequenced.cpp +++ b/clang/test/SemaCXX/warn-unsequenced.cpp @@ -278,6 +278,174 @@ } } +namespace overloaded_operators { + struct E { + E &operator=(E &); + E operator()(E); + E operator()(E, E); + E operator[](E); + } e; + // Binary operators with unsequenced operands. + E operator+(E,E); + E operator-(E,E); + E operator*(E,E); + E operator/(E,E); + E operator%(E,E); + E operator^(E,E); + E operator&(E,E); + E operator|(E,E); + + E operator<(E,E); + E operator>(E,E); + E operator==(E,E); + E operator!=(E,E); + E operator>=(E,E); + E operator<=(E,E); + + // Binary operators where the RHS is sequenced before the LHS in C++17. + E operator+=(E,E); + E operator-=(E,E); + E operator*=(E,E); + E operator/=(E,E); + E operator%=(E,E); + E operator^=(E,E); + E operator&=(E,E); + E operator|=(E,E); + E operator<<=(E,E); + E operator>>=(E,E); + + // Binary operators where the LHS is sequenced before the RHS in C++17. + E operator<<(E,E); + E operator>>(E,E); + E operator&&(E,E); + E operator||(E,E); + E operator,(E,E); + E operator->*(E,E); + + void test() { + int i = 0; + // Binary operators with unsequenced operands. + ((void)i++,e) + ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) - ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) * ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) / ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) % ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) ^ ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) & ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) | ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + + ((void)i++,e) < ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) > ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) == ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) != ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) <= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) >= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + + // Binary operators where the RHS is sequenced before the LHS in C++17. + ((void)i++,e) = ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) += ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) -= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) *= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) /= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) %= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) ^= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) &= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) |= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) <<= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) >>= ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + + operator+=(((void)i++,e), ((void)i++,e)); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + + // Binary operators where the LHS is sequenced before the RHS in C++17. + ((void)i++,e) << ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) >> ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) || ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) && ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e) , ((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + ((void)i++,e)->*((void)i++,e); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + + operator<<(((void)i++,e), ((void)i++,e)); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + + ((void)i++,e)[((void)i++,e)]; + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + + ((void)i++,e)(((void)i++,e)); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + e(((void)i++,e), ((void)i++,e)); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + + ((void)i++,e).operator()(((void)i++,e)); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + + } +} + +namespace PR35340 { + struct S {}; + S &operator<<(S &, int); + + void test() { + S s; + int i = 0; + s << i++ << i++; + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + + operator<<(operator<<(s, i++), i++); + // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}} + // cxx17-warning@-2 {{multiple unsequenced modifications to 'i'}} + } +} + namespace members { struct S1 { @@ -612,7 +780,6 @@ if (static_cast((num = bar.get()) < 5) && static_cast(num < 10)) { } // cxx11-warning@-1 {{unsequenced modification and access to 'num'}} - // cxx17-warning@-2 {{unsequenced modification and access to 'num'}} foo(num++, num++); // cxx11-warning@-1 {{multiple unsequenced modifications to 'num'}} @@ -630,13 +797,11 @@ T t = static_cast(0); return (t = static_cast(1)) && t; // cxx11-warning@-1 {{unsequenced modification and access to 't'}} - // cxx17-warning@-2 {{unsequenced modification and access to 't'}} } int y = Run2(); int z = Run2(); // cxx11-note@-1{{in instantiation of function template specialization 'templates::Run2' requested here}} -// cxx17-note@-2{{in instantiation of function template specialization 'templates::Run2' requested here}} template int var = sizeof(T); void test_var() {