diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2129,6 +2129,8 @@ "multiple unsequenced modifications to %0">, InGroup; def warn_unsequenced_mod_use : Warning< "unsequenced modification and access to %0">, InGroup; +def note_in_default_argument_or_initializer_used_here : Note< + "in default %select{argument|initializer}0 used here">; def select_initialized_entity_kind : TextSubstitution< "%select{copying variable|copying parameter|" 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 @@ -12620,9 +12620,28 @@ const Expr *UsageExpr; SequenceTree::Seq Seq; - Usage() : UsageExpr(nullptr), Seq() {} + /// The index in \p AdditionalSLData for the additional \p SourceLocation + /// data if >= 0, or -1 if there is no additional data (the common case). + int AdditionalSLDataIndex; + + Usage() : UsageExpr(nullptr), Seq(), AdditionalSLDataIndex(-1) {} + }; + + enum ASLD_Kind { ASLD_DefaultArgument, ASLD_DefaultInitializer }; + /// A structure storing extra location data. These structures + /// form a chain in \p AdditionalSLData. + struct AdditionalSourceLocationData { + SourceLocation SL; + int Kind : 2; + int PrevIndex : 30; + AdditionalSourceLocationData(SourceLocation SL, ASLD_Kind K, int PrevIndex) + : SL(SL), Kind(K), PrevIndex(PrevIndex) {} }; + /// One \p AdditionalSourceLocationData is added to this vector + /// for each \p CXXDefaultArgExpr or \p CXXDefaultInitExpr. + SmallVector AdditionalSLData; + struct UsageInfo { Usage Uses[UK_Count]; @@ -12712,6 +12731,30 @@ bool EvalOK = true; } *EvalTracker = nullptr; + /// RAII object used to wrap the visitation of an expression with additional + /// source location data. This is used to keep track of chains of default + /// argument and initializer. + class AdditionalSourceLocationTracker { + public: + AdditionalSourceLocationTracker(SequenceChecker &Self, SourceLocation SL, + ASLD_Kind K) + : Self(Self), Prev(Self.AdditionalSLTracker), + AdditionalSLDataIndex(-1) { + Self.AdditionalSLTracker = this; + Self.AdditionalSLData.emplace_back( + SL, K, Prev ? Prev->AdditionalSLDataIndex : -1); + AdditionalSLDataIndex = Self.AdditionalSLData.size() - 1; + } + + ~AdditionalSourceLocationTracker() { Self.AdditionalSLTracker = Prev; } + int additionalSLDataIndex() const { return AdditionalSLDataIndex; } + + private: + SequenceChecker &Self; + AdditionalSourceLocationTracker *Prev; + int AdditionalSLDataIndex; + } *AdditionalSLTracker = nullptr; + /// Find the object which is produced by the specified expression, /// if any. Object getObject(const Expr *E, bool Mod) const { @@ -12749,6 +12792,11 @@ // Then record the new usage with the current sequencing region. U.UsageExpr = UsageExpr; U.Seq = Region; + + // If we have additional source location data, store the index of + // the head of the location data chain in \p U. + if (AdditionalSLTracker) + U.AdditionalSLDataIndex = AdditionalSLTracker->additionalSLDataIndex(); } } @@ -12776,6 +12824,29 @@ SemaRef.PDiag(IsModMod ? diag::warn_unsequenced_mod_mod : diag::warn_unsequenced_mod_use) << O << SourceRange(ModOrUse->getExprLoc())); + + int Index; + if (OtherKind == UK_Use) { + // Walk back the current chain. + Index = AdditionalSLTracker ? AdditionalSLTracker->additionalSLDataIndex() + : -1; + } else { + // Walk back the stored chain. + Index = U.AdditionalSLDataIndex; + } + + while (Index != -1) { + const AdditionalSourceLocationData &ASLD = AdditionalSLData[Index]; + if (ASLD.SL.isValid()) + SemaRef.DiagRuntimeBehavior( + ASLD.SL, {Mod, ModOrUse}, + SemaRef.PDiag( + diag::note_in_default_argument_or_initializer_used_here) + << ASLD.Kind); + + Index = ASLD.PrevIndex; + } + UI.Diagnosed = true; } @@ -13323,6 +13394,20 @@ }); } + void VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *DIE) { + // Remember that we are visiting this default initializer. + AdditionalSourceLocationTracker SL(*this, DIE->getUsedLocation(), + ASLD_DefaultInitializer); + Visit(DIE->getExpr()); + } + + void VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *DAE) { + // Remember that we are visiting this default argument. + AdditionalSourceLocationTracker SL(*this, DAE->getUsedLocation(), + ASLD_DefaultArgument); + Visit(DAE->getExpr()); + } + 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 @@ -268,6 +268,118 @@ // cxx17-warning@-1 {{unsequenced modification and access to 'pf'}} } +namespace default_arg { +int a; +void f1(int = a, int = a++); // cxx11-warning 2{{unsequenced modification and access to 'a'}} + // cxx17-warning@-1 2{{unsequenced modification and access to 'a'}} + +void f2(int = a++, int = a); // cxx11-warning {{unsequenced modification and access to 'a'}} + // cxx17-warning@-1 {{unsequenced modification and access to 'a'}} + +void f3(int = a++, int = sizeof(a)); + +void test() { + int b; + f1(); // cxx11-note {{in default argument used here}} + // cxx17-note@-1 {{in default argument used here}} + f1(a); // cxx11-note {{in default argument used here}} + // cxx17-note@-1 {{in default argument used here}} + f1(a, a); // ok + f1(b); // ok + f1(b, a++); // ok + + f2(); // cxx11-note {{in default argument used here}} + // cxx17-note@-1 {{in default argument used here}} + f2(a); // ok + f2(a++); // cxx11-warning {{unsequenced modification and access to 'a'}} + // cxx17-warning@-1 {{unsequenced modification and access to 'a'}} + + f3(); // ok + f3(a++); // ok +} +} // namespace default_arg + +namespace default_init { +int a; +struct AggregateInCXX14 { + int b = ++a; // cxx17-warning {{unsequenced modification and access to 'a'}} +}; +void test_default_init() { + int c = AggregateInCXX14{}.b + a; // cxx17-note {{in default initializer used here}} +} + +struct NonAggregate { + int b = ++a; + virtual ~NonAggregate(); +}; +void test_default_init_non_aggregate() { + int c = NonAggregate{}.b + a; +} +} // namespace default_init + +namespace default_arg_chain_0 { +int i; +int f0(int = i); +int f1(int = f0()); +int f2(int = f1()); +int f3(int, int = f2()); + +void test() { + f3(i); // ok + f3(i++); // cxx11-warning {{unsequenced modification and access to 'i'}} + // cxx17-warning@-1 {{unsequenced modification and access to 'i'}} +} +} // namespace default_arg_chain_0 + +namespace default_arg_chain_1 { +int i; +int f0(int = i++); // cxx11-warning {{unsequenced modification and access to 'i'}} + // cxx17-warning@-1 {{unsequenced modification and access to 'i'}} + +int f1(int = f0()); // cxx11-note {{in default argument used here}} + // cxx17-note@-1 {{in default argument used here}} + +int f2(int = f1()); // cxx11-note {{in default argument used here}} + // cxx17-note@-1 {{in default argument used here}} + +int f3(int, int = f2()); // cxx11-note {{in default argument used here}} + // cxx17-note@-1 {{in default argument used here}} + +void test() { + f3(0); // ok + f3(i); // cxx11-note {{in default argument used here}} + // cxx17-note@-1 {{in default argument used here}} +} +} // namespace default_arg_chain_1 + +namespace default_arg_init_chain { +int i; +struct X { + int b = ++i; // cxx17-warning 4{{unsequenced modification and access to 'i'}} +}; + +// Ok since X is an aggregate since C++14. + +int g0(int = X{}.b); // cxx17-note 4{{in default initializer used here}} +int g1(int = g0()); // cxx17-note 4{{in default argument used here}} +int g2(int = g1()); // cxx17-note 4{{in default argument used here}} +int g3(int = g2()); // cxx17-note 4{{in default argument used here}} + +int g4a(int, int = g3() + i); // cxx17-note {{in default argument used here}} +int g4b(int, int = i + g3()); // cxx17-note {{in default argument used here}} + +int g5a(int = 0, int = g3()); // cxx17-note {{in default argument used here}} +int g5b(int = g3(), int = 0); + +void test() { + g5a(); // ok + g5b(); // ok + g5a(i); // cxx17-note {{in default argument used here}} + g5b(g3()); // ok + g5b(g3(), i); // cxx17-note {{in default argument used here}} +} +} // namespace default_arg_init_chain + namespace PR20819 { struct foo { void bar(int); }; foo get_foo(int);