riccibruno updated this revision to Diff 271659.
riccibruno added a comment.
- Remove a left-over FIXME in the tests.
- Use a `%select` in the diagnostic.
- Pack `AdditionalSourceLocationData` into two `unsigned`s.
- Simplify the chain walking code in `checkUsage`.
- Mark the patch as needing review since the code has changed substantially
since the original patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81003/new/
https://reviews.llvm.org/D81003
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
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
@@ -268,6 +268,114 @@
// 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_init {
+ int a;
+ struct AggregateInCXX17 { int b = ++a; };
+ // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+ void test_default_init() {
+ int c = AggregateInCXX17{}.b + a;
+ // cxx17-note@-1 {{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_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_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_init_chain {
+ int i;
+ struct X { int b = ++i; }; // cxx17-warning 4{{unsequenced modification and access to 'i'}}
+
+ // Ok since X is an aggregate in C++17 but not in C++11.
+
+ 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 PR20819 {
struct foo { void bar(int); };
foo get_foo(int);
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12434,9 +12434,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<AdditionalSourceLocationData, 2> AdditionalSLData;
+
struct UsageInfo {
Usage Uses[UK_Count];
@@ -12526,6 +12545,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 {
@@ -12563,6 +12606,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();
}
}
@@ -12590,6 +12638,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;
}
@@ -13013,6 +13084,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);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2119,6 +2119,8 @@
"multiple unsequenced modifications to %0">, InGroup<Unsequenced>;
def warn_unsequenced_mod_use : Warning<
"unsequenced modification and access to %0">, InGroup<Unsequenced>;
+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|"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
