riccibruno created this revision.
riccibruno added a reviewer: rsmith.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
riccibruno added a parent revision: D81003: [clang][Sema] SequenceChecker: Also
visit default arguments..
In C++17 the operand(s) of an overloaded operator are sequenced as for the
corresponding built-in operator when the overloaded operator is called with the
operator notation ([over.match.oper]p2).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81330
Files:
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/warn-unsequenced.cpp
Index: clang/test/SemaCXX/warn-unsequenced.cpp
===================================================================
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -308,6 +308,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 {
@@ -642,7 +810,6 @@
if (static_cast<E>((num = bar.get()) < 5) && static_cast<E>(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'}}
@@ -660,13 +827,11 @@
T t = static_cast<T>(0);
return (t = static_cast<T>(1)) && t;
// cxx11-warning@-1 {{unsequenced modification and access to 't'}}
- // cxx17-warning@-2 {{unsequenced modification and access to 't'}}
}
int y = Run2<bool>();
int z = Run2<E>();
// cxx11-note@-1{{in instantiation of function template specialization 'templates::Run2<templates::E>' requested here}}
-// cxx17-note@-2{{in instantiation of function template specialization 'templates::Run2<templates::E>' requested here}}
template <typename T> int var = sizeof(T);
void test_var() {
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12981,6 +12981,130 @@
Visit(DAE->getExpr());
}
+ 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<const Expr *> 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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
