riccibruno created this revision.
riccibruno added reviewers: aaron.ballman, rsmith.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.

Teach the unsequenced operation checker about references and members. To do 
this introduce a class `MemoryLocation` which will approximate C++ memory 
locations.

We don't handle bit-fields for now but I plan to do this later. I also plan to 
teach it about the C++17 sequencing rules, and fix a number of corner cases 
around `||`, `&&` and `?:`.


Repository:
  rC Clang

https://reviews.llvm.org/D57660

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-unsequenced.cpp

Index: test/SemaCXX/warn-unsequenced.cpp
===================================================================
--- test/SemaCXX/warn-unsequenced.cpp
+++ test/SemaCXX/warn-unsequenced.cpp
@@ -131,54 +131,53 @@
 
   // Operations with a single member of the implicit object.
   ++a = 0; // no-warning
-  a + ++a; // expected-warning {{unsequenced modification and access to 'a'}}
+  a + ++a; // expected-warning {{unsequenced modification and access to member 'a'}}
   a = ++a; // no-warning
-  a + a++; // expected-warning {{unsequenced modification and access to 'a'}}
-  a = a++; // expected-warning {{multiple unsequenced modifications to 'a'}}
+  a + a++; // expected-warning {{unsequenced modification and access to member 'a'}}
+  a = a++; // expected-warning {{multiple unsequenced modifications to member 'a'}}
   ++ ++a; // no-warning
   (a++, a++); // no-warning
-  ++a + ++a; // expected-warning {{multiple unsequenced modifications to 'a'}}
-  a++ + a++; // expected-warning {{multiple unsequenced modifications to 'a'}}
+  ++a + ++a; // expected-warning {{multiple unsequenced modifications to member 'a'}}
+  a++ + a++; // expected-warning {{multiple unsequenced modifications to member 'a'}}
   (a++, a) = 0; // no-warning
   a = xs[++a]; // no-warning
-  a = xs[a++]; // expected-warning {{multiple unsequenced modifications to 'a'}}
-  (a ? xs[0] : xs[1]) = ++a; // expected-warning {{unsequenced modification and access to 'a'}}
+  a = xs[a++]; // expected-warning {{multiple unsequenced modifications to member 'a'}}
+  (a ? xs[0] : xs[1]) = ++a; // expected-warning {{unsequenced modification and access to member 'a'}}
   a = (++a, ++a); // no-warning
   a = (a++, ++a); // no-warning
-  a = (a++, a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
+  a = (a++, a++); // expected-warning {{multiple unsequenced modifications to member 'a'}}
   f(a, a); // no-warning
-  f(a = 0, a); // expected-warning {{unsequenced modification and access to 'a'}}
-  f(a, a += 0); // expected-warning {{unsequenced modification and access to 'a'}}
-  f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications to 'a'}}
+  f(a = 0, a); // expected-warning {{unsequenced modification and access to member 'a'}}
+  f(a, a += 0); // expected-warning {{unsequenced modification and access to member 'a'}}
+  f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications to member 'a'}}
   a = f(++a); // no-warning
   a = f(a++); // no-warning
-  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
+  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications to member 'a'}}
 
   // Operations with a single member of the other object 's'.
-  // TODO: For now this is completely unhandled.
   ++s.a = 0; // no-warning
-  s.a + ++s.a; // no-warning TODO {{unsequenced modification and access to }}
+  s.a + ++s.a; // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
   s.a = ++s.a; // no-warning
-  s.a + s.a++; // no-warning TODO {{unsequenced modification and access to }}
-  s.a = s.a++; // no-warning TODO {{multiple unsequenced modifications to }}
+  s.a + s.a++; // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
+  s.a = s.a++; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   ++ ++s.a; // no-warning
   (s.a++, s.a++); // no-warning
-  ++s.a + ++s.a; // no-warning TODO {{multiple unsequenced modifications to }}
-  s.a++ + s.a++; // no-warning TODO {{multiple unsequenced modifications to }}
+  ++s.a + ++s.a; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+  s.a++ + s.a++; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   (s.a++, s.a) = 0; // no-warning
   s.a = xs[++s.a]; // no-warning
-  s.a = xs[s.a++]; // no-warning TODO {{multiple unsequenced modifications to }}
-  (s.a ? xs[0] : xs[1]) = ++s.a; // no-warning TODO {{unsequenced modification and access to }}
+  s.a = xs[s.a++]; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+  (s.a ? xs[0] : xs[1]) = ++s.a; // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
   s.a = (++s.a, ++s.a); // no-warning
   s.a = (s.a++, ++s.a); // no-warning
-  s.a = (s.a++, s.a++); // no-warning TODO {{multiple unsequenced modifications to }}
+  s.a = (s.a++, s.a++); // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   f(s.a, s.a); // no-warning
-  f(s.a = 0, s.a); // no-warning TODO {{unsequenced modification and access to }}
-  f(s.a, s.a += 0); // no-warning TODO {{unsequenced modification and access to }}
-  f(s.a = 0, s.a = 0); // no-warning TODO {{multiple unsequenced modifications to }}
+  f(s.a = 0, s.a); // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
+  f(s.a, s.a += 0); // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
+  f(s.a = 0, s.a = 0); // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   s.a = f(++s.a); // no-warning
   s.a = f(s.a++); // no-warning
-  s.a = f(++s.a, s.a++); // no-warning TODO {{multiple unsequenced modifications to }}
+  s.a = f(++s.a, s.a++); // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
 
   // Operations with two distinct members of the implicit object. All of them are fine.
   a + ++b; // no-warning
@@ -259,8 +258,7 @@
 
 namespace references {
 void reference_f() {
-  // TODO: Check that we can see through references.
-  // For now this is completely unhandled.
+  // Check that we can see through references.
   int a;
   int xs[10];
   int &b = a;
@@ -269,28 +267,28 @@
   int &ra1 = c;
   int &ra2 = b;
   ++ra1 = 0; // no-warning
-  ra1 + ++ra2; // no-warning TODO: {{unsequenced modification and access to 'a'}}
+  ra1 + ++ra2; // expected-warning {{unsequenced modification and access to 'a'}}
   ra1 = ++ra2; // no-warning
-  ra1 + ra2++; // no-warning TODO: {{unsequenced modification and access to 'a'}}
-  ra1 = ra2++; // no-warning TODO: {{multiple unsequenced modifications to 'a'}}
+  ra1 + ra2++; // expected-warning {{unsequenced modification and access to 'a'}}
+  ra1 = ra2++; // expected-warning {{multiple unsequenced modifications to 'a'}}
   ++ ++ra2; // no-warning
   (ra1++, ra2++); // no-warning
-  ++ra1 + ++ra2; // no-warning TODO: {{multiple unsequenced modifications to 'a'}}
-  ra1++ + ra2++; // no-warning TODO: {{multiple unsequenced modifications to 'a'}}
+  ++ra1 + ++ra2; // expected-warning {{multiple unsequenced modifications to 'a'}}
+  ra1++ + ra2++; // expected-warning {{multiple unsequenced modifications to 'a'}}
   (ra1++, ra2) = 0; // no-warning
   ra1 = xs[++ra2]; // no-warning
-  ra1 = xs[ra2++]; // no-warning TODO: {{multiple unsequenced modifications to 'a'}}
-  (ra1 ? xs[0] : xs[1]) = ++ra2; // no-warning TODO: {{unsequenced modification and access to 'a'}}
+  ra1 = xs[ra2++]; // expected-warning {{multiple unsequenced modifications to 'a'}}
+  (ra1 ? xs[0] : xs[1]) = ++ra2; // expected-warning {{unsequenced modification and access to 'a'}}
   ra1 = (++ra2, ++ra2); // no-warning
   ra1 = (ra2++, ++ra2); // no-warning
-  ra1 = (ra2++, ra2++); // no-warning TODO: {{multiple unsequenced modifications to 'a'}}
+  ra1 = (ra2++, ra2++); // expected-warning {{multiple unsequenced modifications to 'a'}}
   f(ra1, ra2); // no-warning
-  f(ra1 = 0, ra2); // no-warning TODO: {{unsequenced modification and access to 'a'}}
-  f(ra1, ra2 += 0); // no-warning TODO: {{unsequenced modification and access to 'a'}}
-  f(ra1 = 0, ra2 = 0); // no-warning TODO: {{multiple unsequenced modifications to 'a'}}
+  f(ra1 = 0, ra2); // expected-warning {{unsequenced modification and access to 'a'}}
+  f(ra1, ra2 += 0); // expected-warning {{unsequenced modification and access to 'a'}}
+  f(ra1 = 0, ra2 = 0); // expected-warning {{multiple unsequenced modifications to 'a'}}
   ra1 = f(++ra2); // no-warning
   ra1 = f(ra2++); // no-warning
-  ra1 = f(++ra2, ra1++); // no-warning TODO: {{multiple unsequenced modifications to 'a'}}
+  ra1 = f(++ra2, ra1++); // expected-warning {{multiple unsequenced modifications to 'a'}}
 
   // Make sure we don't crash if we have a reference cycle.
   int &ref_cycle = ref_cycle;
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11641,8 +11641,83 @@
 
 namespace {
 
+/// An approximation of a C++ memory location used by SequenceChecker.
+/// TODO: Handle bit-fields.
+/// TODO: Give better info with references.
+class MemoryLocation {
+  friend struct llvm::DenseMapInfo<MemoryLocation>;
+  /// An object. As a special case this represent the C++ implicit object
+  /// if the ValueDecl * is null and the flag is true.
+  llvm::PointerIntPair<ValueDecl *, 1> ObjectOrCXXThis;
+  /// If non-null, a field in a record type.
+  ValueDecl *Field = nullptr;
+
+public:
+  struct CXXThisTag {};
+  MemoryLocation() = default;
+  MemoryLocation(ValueDecl *Object, ValueDecl *Field)
+      : ObjectOrCXXThis(Object, /*IsCXXThis=*/false), Field(Field) {}
+  MemoryLocation(CXXThisTag, ValueDecl *Field)
+      : ObjectOrCXXThis(nullptr, /*IsCXXThis=*/true), Field(Field) {}
+  MemoryLocation(void *OpaqueObject, ValueDecl *Field) : Field(Field) {
+    ObjectOrCXXThis.setFromOpaqueValue(OpaqueObject);
+  }
+
+  explicit operator bool() const { return getObject() || isCXXThis(); }
+  ValueDecl *getObject() const { return ObjectOrCXXThis.getPointer(); }
+  bool isCXXThis() const { return ObjectOrCXXThis.getInt(); }
+  ValueDecl *getField() const { return Field; }
+  void *getOpaqueObject() const { return ObjectOrCXXThis.getOpaqueValue(); }
+
+  friend bool operator==(const MemoryLocation &MemoryLoc1,
+                         const MemoryLocation &MemoryLoc2) {
+    return !(MemoryLoc1 != MemoryLoc2);
+  }
+  friend bool operator!=(const MemoryLocation &MemoryLoc1,
+                         const MemoryLocation &MemoryLoc2) {
+    return (MemoryLoc1.ObjectOrCXXThis != MemoryLoc2.ObjectOrCXXThis) ||
+           (MemoryLoc1.Field != MemoryLoc2.Field);
+  }
+}; // class MemoryLocation
+
+} // namespace
+
+namespace llvm {
+
+template <> struct llvm::DenseMapInfo<MemoryLocation> {
+  using FirstTy = llvm::PointerIntPair<ValueDecl *, 1>;
+  using SecondTy = ValueDecl *;
+  using FirstInfo = llvm::DenseMapInfo<FirstTy>;
+  using SecondInfo = llvm::DenseMapInfo<SecondTy>;
+  using PairInfo = llvm::DenseMapInfo<std::pair<FirstTy, SecondTy>>;
+
+  static MemoryLocation getEmptyKey() {
+    return MemoryLocation(FirstInfo::getEmptyKey().getOpaqueValue(),
+                          SecondInfo::getEmptyKey());
+  }
+
+  static MemoryLocation getTombstoneKey() {
+    return MemoryLocation(FirstInfo::getTombstoneKey().getOpaqueValue(),
+                          SecondInfo::getTombstoneKey());
+  }
+
+  static unsigned getHashValue(const MemoryLocation &MemoryLoc) {
+    return PairInfo::getHashValue(std::pair<FirstTy, SecondTy>(
+        MemoryLoc.ObjectOrCXXThis, MemoryLoc.Field));
+  }
+
+  static bool isEqual(const MemoryLocation &MemoryLoc1,
+                      const MemoryLocation &MemoryLoc2) {
+    return MemoryLoc1 == MemoryLoc2;
+  }
+};
+
+} // namespace llvm
+
+namespace {
+
 /// Visitor for expressions which looks for unsequenced operations on the
-/// same object.
+/// same memory location.
 class SequenceChecker : public EvaluatedExprVisitor<SequenceChecker> {
   using Base = EvaluatedExprVisitor<SequenceChecker>;
 
@@ -11713,21 +11788,18 @@
     }
   };
 
-  /// An object for which we can track unsequenced uses.
-  using Object = NamedDecl *;
-
-  /// Different flavors of object usage which we track. We only track the
-  /// least-sequenced usage of each kind.
+  /// Different flavors of memory location usage which we track. We only
+  /// track the least-sequenced usage of each kind.
   enum UsageKind {
-    /// A read of an object. Multiple unsequenced reads are OK.
+    /// A read of a memory location. Multiple unsequenced reads are OK.
     UK_Use,
 
-    /// A modification of an object which is sequenced before the value
+    /// A modification of a memory location which is sequenced before the value
     /// computation of the expression, such as ++n in C++.
     UK_ModAsValue,
 
-    /// A modification of an object which is not sequenced before the value
-    /// computation of the expression, such as n++.
+    /// A modification of an memory location which is not sequenced before the
+    /// value computation of the expression, such as n++.
     UK_ModAsSideEffect,
 
     UK_Count = UK_ModAsSideEffect + 1
@@ -11750,7 +11822,7 @@
     /// The sequencing regions corresponding to each usage kind.
     SequenceTree::Seq Seqs[UK_Count]{};
 
-    /// Have we issued a diagnostic for this object already?
+    /// Have we issued a diagnostic for this memory location already?
     bool Diagnosed = false;
 
   public:
@@ -11766,7 +11838,7 @@
     void markDiagnosed() { Diagnosed = true; }
     bool alreadyDiagnosed() const { return Diagnosed; }
   };
-  using UsageInfoMap = llvm::SmallDenseMap<Object, UsageInfo, 16>;
+  using UsageInfoMap = llvm::SmallDenseMap<MemoryLocation, UsageInfo, 16>;
 
   Sema &SemaRef;
 
@@ -11781,7 +11853,7 @@
 
   /// Filled in with declarations which were modified as a side-effect
   /// (that is, post-increment operations).
-  SmallVectorImpl<std::pair<Object, Usage>> *ModAsSideEffect = nullptr;
+  SmallVectorImpl<std::pair<MemoryLocation, Usage>> *ModAsSideEffect = nullptr;
 
   /// Expressions to check later. We defer checking these to reduce
   /// stack usage.
@@ -11799,7 +11871,8 @@
     }
 
     ~SequencedSubexpression() {
-      for (std::pair<Object, Usage> &M : llvm::reverse(ModAsSideEffect)) {
+      for (std::pair<MemoryLocation, Usage> &M :
+           llvm::reverse(ModAsSideEffect)) {
         // Add a new usage with usage kind UK_ModAsValue, and then restore
         // the previous usage with UK_ModAsSideEffect (thus clearing it if
         // the previous one was empty).
@@ -11812,8 +11885,8 @@
     }
 
     SequenceChecker &Self;
-    SmallVector<std::pair<Object, Usage>, 4> ModAsSideEffect;
-    SmallVectorImpl<std::pair<Object, Usage>> *OldModAsSideEffect;
+    SmallVector<std::pair<MemoryLocation, Usage>, 4> ModAsSideEffect;
+    SmallVectorImpl<std::pair<MemoryLocation, Usage>> *OldModAsSideEffect;
   };
 
   /// RAII object wrapping the visitation of a subexpression which we might
@@ -11846,48 +11919,81 @@
     bool EvalOK = true;
   } *EvalTracker = nullptr;
 
-  /// Find the object which is produced by the specified expression,
+  /// Find the memory location which is produced by the specified expression,
   /// if any.
-  Object getObject(Expr *E, bool Mod) const {
+  static MemoryLocation getMemoryLocation(Expr *E, bool Mod) {
+    // Hold the references we have seen. This is needed to avoid a
+    // cycle such as in "int &i = i;". Most of the time this set will
+    // have size < 2.
+    llvm::SmallPtrSet<VarDecl *, 2> RefsSeen;
+    return getMemoryLocationImpl(E, Mod, RefsSeen);
+  }
+
+  static MemoryLocation
+  getMemoryLocationImpl(Expr *E, bool Mod,
+                        llvm::SmallPtrSetImpl<VarDecl *> &RefsSeen) {
     E = E->IgnoreParenCasts();
-    if (UnaryOperator *UO = dyn_cast<UnaryOperator>(E)) {
+    if (auto *UO = dyn_cast<UnaryOperator>(E)) {
       if (Mod && (UO->getOpcode() == UO_PreInc || UO->getOpcode() == UO_PreDec))
-        return getObject(UO->getSubExpr(), Mod);
-    } else if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
+        return getMemoryLocationImpl(UO->getSubExpr(), Mod, RefsSeen);
+    }
+
+    else if (auto *BO = dyn_cast<BinaryOperator>(E)) {
       if (BO->getOpcode() == BO_Comma)
-        return getObject(BO->getRHS(), Mod);
+        return getMemoryLocationImpl(BO->getRHS(), Mod, RefsSeen);
       if (Mod && BO->isAssignmentOp())
-        return getObject(BO->getLHS(), Mod);
-    } else if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
-      // FIXME: Check for more interesting cases, like "x.n = ++x.n".
-      if (isa<CXXThisExpr>(ME->getBase()->IgnoreParenCasts()))
-        return ME->getMemberDecl();
-    } else if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
-      // FIXME: If this is a reference, map through to its value.
-      return DRE->getDecl();
-    return nullptr;
+        return getMemoryLocationImpl(BO->getLHS(), Mod, RefsSeen);
+    }
+
+    else if (auto *ME = dyn_cast<MemberExpr>(E)) {
+      ValueDecl *Field = ME->getMemberDecl();
+      MemoryLocation BaseMemoryLoc =
+          getMemoryLocationImpl(ME->getBase(), Mod, RefsSeen);
+      return MemoryLocation(BaseMemoryLoc.getOpaqueObject(), Field);
+    }
+
+    else if (auto *CXXTE = dyn_cast<CXXThisExpr>(E)) {
+      return MemoryLocation(MemoryLocation::CXXThisTag(), /*Field=*/nullptr);
+    }
+
+    else if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+      if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+        // If this is a reference, look through it.
+        if (VD->getType()->isReferenceType() && VD->hasInit()) {
+          // But only if we don't have a cycle.
+          bool Inserted = RefsSeen.insert(VD).second;
+          if (Inserted)
+            return getMemoryLocationImpl(VD->getInit(), Mod, RefsSeen);
+        }
+        return MemoryLocation(VD, /*Field=*/nullptr);
+      }
+    }
+    return MemoryLocation();
   }
 
-  /// Note that an object was modified or used by an expression.
-  /// UI is the UsageInfo for the object O as obtained via the UsageMap.
-  void addUsage(Object O, UsageInfo &UI, Expr *UsageExpr, UsageKind UK) {
-    // Get the old usage for the given object and usage kind.
+  /// Note that a memory location was modified or used by an expression.
+  /// UI is the UsageInfo for the memory location MemoryLoc as obtained
+  /// via the UsageMap.
+  void addUsage(MemoryLocation MemoryLoc, UsageInfo &UI, Expr *UsageExpr,
+                UsageKind UK) {
+    // Get the old usage for the given memory location and usage kind.
     Usage U = UI.getUsage(UK);
     if (!U.UsageExpr || !Tree.isUnsequenced(Region, U.Seq)) {
       // If we have a modification as side effect and are in a sequenced
       // subexpression, save the old Usage so that we can restore it later
       // in SequencedSubexpression::~SequencedSubexpression.
       if (UK == UK_ModAsSideEffect && ModAsSideEffect)
-        ModAsSideEffect->push_back(std::make_pair(O, U));
+        ModAsSideEffect->push_back(std::make_pair(MemoryLoc, U));
       // Then record the new usage with the current sequencing region.
       UI.setUsage(UK, Usage(UsageExpr, Region));
     }
   }
 
   /// Check whether a modification or use conflicts with a prior usage.
-  /// UI is the UsageInfo for the object O as obtained via the UsageMap.
-  void checkUsage(Object O, UsageInfo &UI, Expr *UsageExpr, UsageKind OtherKind,
-                  bool IsModMod) {
+  /// UI is the UsageInfo for the memory location MemoryLoc as obtained
+  /// via the UsageMap.
+  void checkUsage(MemoryLocation MemoryLoc, UsageInfo &UI, Expr *UsageExpr,
+                  UsageKind OtherKind, bool IsModMod) {
     if (UI.alreadyDiagnosed())
       return;
 
@@ -11900,10 +12006,14 @@
     if (OtherKind == UK_Use)
       std::swap(Mod, ModOrUse);
 
-    SemaRef.Diag(Mod->getExprLoc(),
-                 IsModMod ? diag::warn_unsequenced_mod_mod
-                          : diag::warn_unsequenced_mod_use)
-      << O << SourceRange(ModOrUse->getExprLoc());
+    bool IsMember = MemoryLoc.getField() != nullptr;
+    bool KnownObject = MemoryLoc.getObject() != nullptr;
+
+    SemaRef.Diag(Mod->getExprLoc(), IsModMod ? diag::warn_unsequenced_mod_mod
+                                             : diag::warn_unsequenced_mod_use)
+        << IsMember << (KnownObject && IsMember)
+        << (IsMember ? MemoryLoc.getField() : MemoryLoc.getObject())
+        << MemoryLoc.getObject() << SourceRange(ModOrUse->getExprLoc());
     UI.markDiagnosed();
   }
 
@@ -11913,11 +12023,12 @@
   //  "((++k)++, k) = k" or "k = (k++, k++)". Both contain unsequenced
   //  operations before C++17 and both are well-defined in C++17).
   //
-  // When visiting a node which uses/modify an object we first call notePreUse
-  // or notePreMod before visiting its sub-expression(s). At this point the
-  // children of the current node have not yet been visited and so the eventual
-  // uses/modifications resulting from the children of the current node have not
-  // been recorded yet.
+  // When visiting a node which uses/modify a memory location we first call
+  // notePreUse or notePreMod before visiting its sub-expression(s). At this
+  // point the children of the current node have not yet been visited and so
+  // the eventual uses/modifications resulting from the children of the current
+  // node have not been recorded yet. They will therefore not be wrongly
+  // detected by the call(s) to checkUsage.
   //
   // We then visit the children of the current node. After that notePostUse or
   // notePostMod is called. These will 1) detect an unsequenced modification
@@ -11933,31 +12044,34 @@
   // modification as side effect) when exiting the scope of the sequenced
   // subexpression.
 
-  void notePreUse(Object O, Expr *UseExpr) {
-    UsageInfo &UI = UsageMap[O];
+  void notePreUse(MemoryLocation MemoryLoc, Expr *UseExpr) {
+    UsageInfo &UI = UsageMap[MemoryLoc];
     // Uses conflict with other modifications.
-    checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_ModAsValue, /*IsModMod=*/false);
+    checkUsage(MemoryLoc, UI, UseExpr, /*OtherKind=*/UK_ModAsValue,
+               /*IsModMod=*/false);
   }
 
-  void notePostUse(Object O, Expr *UseExpr) {
-    UsageInfo &UI = UsageMap[O];
-    checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_ModAsSideEffect,
+  void notePostUse(MemoryLocation MemoryLoc, Expr *UseExpr) {
+    UsageInfo &UI = UsageMap[MemoryLoc];
+    checkUsage(MemoryLoc, UI, UseExpr, /*OtherKind=*/UK_ModAsSideEffect,
                /*IsModMod=*/false);
-    addUsage(O, UI, UseExpr, /*UsageKind=*/UK_Use);
+    addUsage(MemoryLoc, UI, UseExpr, /*UsageKind=*/UK_Use);
   }
 
-  void notePreMod(Object O, Expr *ModExpr) {
-    UsageInfo &UI = UsageMap[O];
+  void notePreMod(MemoryLocation MemoryLoc, Expr *ModExpr) {
+    UsageInfo &UI = UsageMap[MemoryLoc];
     // Modifications conflict with other modifications and with uses.
-    checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsValue, /*IsModMod=*/true);
-    checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_Use, /*IsModMod=*/false);
+    checkUsage(MemoryLoc, UI, ModExpr, /*OtherKind=*/UK_ModAsValue,
+               /*IsModMod=*/true);
+    checkUsage(MemoryLoc, UI, ModExpr, /*OtherKind=*/UK_Use,
+               /*IsModMod=*/false);
   }
 
-  void notePostMod(Object O, Expr *ModExpr, UsageKind UK) {
-    UsageInfo &UI = UsageMap[O];
-    checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsSideEffect,
+  void notePostMod(MemoryLocation MemoryLoc, Expr *ModExpr, UsageKind UK) {
+    UsageInfo &UI = UsageMap[MemoryLoc];
+    checkUsage(MemoryLoc, UI, ModExpr, /*OtherKind=*/UK_ModAsSideEffect,
                /*IsModMod=*/true);
-    addUsage(O, UI, ModExpr, /*UsageKind=*/UK);
+    addUsage(MemoryLoc, UI, ModExpr, /*UsageKind=*/UK);
   }
 
 public:
@@ -11976,15 +12090,15 @@
   }
 
   void VisitCastExpr(CastExpr *E) {
-    Object O = Object();
+    MemoryLocation MemoryLoc;
     if (E->getCastKind() == CK_LValueToRValue)
-      O = getObject(E->getSubExpr(), false);
+      MemoryLoc = getMemoryLocation(E->getSubExpr(), /*Mod=*/false);
 
-    if (O)
-      notePreUse(O, E);
+    if (MemoryLoc)
+      notePreUse(MemoryLoc, E);
     VisitExpr(E);
-    if (O)
-      notePostUse(O, E);
+    if (MemoryLoc)
+      notePostUse(MemoryLoc, E);
   }
 
   void VisitSequencedExpressions(Expr *SequencedBefore, Expr *SequencedAfter) {
@@ -12029,11 +12143,11 @@
     // The modification is sequenced after the value computation of the LHS
     // and RHS, so check it before inspecting the operands and update the
     // map afterwards.
-    Object O = getObject(BO->getLHS(), true);
-    if (!O)
+    MemoryLocation MemoryLoc = getMemoryLocation(BO->getLHS(), /*Mod=*/true);
+    if (!MemoryLoc)
       return VisitExpr(BO);
 
-    notePreMod(O, BO);
+    notePreMod(MemoryLoc, BO);
 
     // C++11 [expr.ass]p7:
     //   E1 op= E2 is equivalent to E1 = E1 op E2, except that E1 is evaluated
@@ -12042,12 +12156,12 @@
     // Therefore, for a compound assignment operator, O is considered used
     // everywhere except within the evaluation of E1 itself.
     if (isa<CompoundAssignOperator>(BO))
-      notePreUse(O, BO);
+      notePreUse(MemoryLoc, BO);
 
     Visit(BO->getLHS());
 
     if (isa<CompoundAssignOperator>(BO))
-      notePostUse(O, BO);
+      notePostUse(MemoryLoc, BO);
 
     Visit(BO->getRHS());
 
@@ -12055,8 +12169,9 @@
     //   the assignment is sequenced [...] before the value computation of the
     //   assignment expression.
     // C11 6.5.16/3 has no such rule.
-    notePostMod(O, BO, SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
-                                                       : UK_ModAsSideEffect);
+    notePostMod(MemoryLoc, BO,
+                SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
+                                                : UK_ModAsSideEffect);
   }
 
   void VisitCompoundAssignOperator(CompoundAssignOperator *CAO) {
@@ -12066,28 +12181,31 @@
   void VisitUnaryPreInc(UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }
   void VisitUnaryPreDec(UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }
   void VisitUnaryPreIncDec(UnaryOperator *UO) {
-    Object O = getObject(UO->getSubExpr(), true);
-    if (!O)
+    MemoryLocation MemoryLoc =
+        getMemoryLocation(UO->getSubExpr(), /*Mod=*/true);
+    if (!MemoryLoc)
       return VisitExpr(UO);
 
-    notePreMod(O, UO);
+    notePreMod(MemoryLoc, UO);
     Visit(UO->getSubExpr());
     // C++11 [expr.pre.incr]p1:
     //   the expression ++x is equivalent to x+=1
-    notePostMod(O, UO, SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
-                                                       : UK_ModAsSideEffect);
+    notePostMod(MemoryLoc, UO,
+                SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
+                                                : UK_ModAsSideEffect);
   }
 
   void VisitUnaryPostInc(UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }
   void VisitUnaryPostDec(UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }
   void VisitUnaryPostIncDec(UnaryOperator *UO) {
-    Object O = getObject(UO->getSubExpr(), true);
-    if (!O)
+    MemoryLocation MemoryLoc =
+        getMemoryLocation(UO->getSubExpr(), /*Mod=*/true);
+    if (!MemoryLoc)
       return VisitExpr(UO);
 
-    notePreMod(O, UO);
+    notePreMod(MemoryLoc, UO);
     Visit(UO->getSubExpr());
-    notePostMod(O, UO, UK_ModAsSideEffect);
+    notePostMod(MemoryLoc, UO, UK_ModAsSideEffect);
   }
 
   /// Don't visit the RHS of '&&' or '||' if it might not be evaluated.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1923,9 +1923,11 @@
   "parenthesized initializer list">;
 
 def warn_unsequenced_mod_mod : Warning<
-  "multiple unsequenced modifications to %0">, InGroup<Unsequenced>;
+  "multiple unsequenced modifications to %select{|member }0%2 "
+  "%select{|of %3}1">, InGroup<Unsequenced>;
 def warn_unsequenced_mod_use : Warning<
-  "unsequenced modification and access to %0">, InGroup<Unsequenced>;
+  "unsequenced modification and access to %select{|member }0%2 "
+  "%select{|of %3}1">, InGroup<Unsequenced>;
 
 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

Reply via email to