riccibruno updated this revision to Diff 281999.
riccibruno edited the summary of this revision.
riccibruno added a comment.

Rebased with a few clang-format fixes.


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,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);
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ 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<AdditionalSourceLocationData, 2> 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);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2129,6 +2129,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
  • [PATCH] D81003: [clang] Sequen... Bruno Ricci via Phabricator via cfe-commits

Reply via email to