[Sema] SequenceChecker: Handle references, members and structured bindings.

[riccibruno]

Summary

Currently the checker for unsequenced operations is not able to see through members and references. For example the following code is not detected:

struct S { unsigned x; } s;
s.x + s.x++;

Improve this by teaching the unsequenced operation checker about references, members (including static members) and structured bindings. To do this introduce a class MemoryLocation which will approximate C++ memory locations. Then modify the method getObject (renamed to getMemoryLocation) to follow references, handle members and look through bindings. Also update the diagnostic messages.

With this patch the sequence checker do not handle bit-fields in any particular way, but ideally we should take into account that two distinct bit-fields can be in the same memory location. This is marked as TODO.

[riccibruno]

Added tests for nested/anonymous unions and local structs.

[riccibruno]

Rebased. Does this implementation make sense ?

[riccibruno]

This needs more work.

[riccibruno]

Added some context in the description of the patch. Ping !

[riccibruno]

Friendly ping. One thing I am wondering about is whether MemoryLocation and getMemoryLocation is duplicating something that is already present somewhere else. It feels like something similar should already exist but I can't find anything (but that is not saying much).

[riccibruno]

Ping.

[riccibruno]

Friendly ping.

[xbolva00]

Added some reviewers

Patch improves suboptimal diagnostic, which misses bugs like:
https://bugs.llvm.org/show_bug.cgi?id=43052

[riccibruno]

In D57660#1637220, @xbolva00 wrote:

Added some reviewers

Patch improves suboptimal diagnostic, which misses bugs like:
https://bugs.llvm.org/show_bug.cgi?id=43052

Thanks for taking a look !

[riccibruno]

Rebased

[Mordante]

I like this improvement. However I'm not a reviewer.

clang/lib/Sema/SemaChecking.cpp

12775

Please use \p here since it's not a parameter definition.

[xbolva00]

clang/lib/Sema/SemaChecking.cpp

13059–13060

You can land some NFC changes in separate commit.

[riccibruno]

In D57660#1764337, @Mordante wrote:

I like this improvement. However I'm not a reviewer.

Thanks for looking at the patch!

You can land some NFC changes in separate commit.

Yep, indeed. Will do when I rebase it.

[xbolva00]

Hello @aaron.ballman / @rsmith, can you please take a look on this patch?

[riccibruno]

I have factored out various NFCs which were present in this patch. This should make review easier.
Also addressed some inline comments.

[riccibruno]

Note that I am not entirely sure that the implementation in getMemoryLocation/MemoryLocation is the right way to do this; I think that this patch should be considered a work-in-progress.

[riccibruno]

Add missing patch context

[rsmith]

clang/include/clang/Basic/DiagnosticSemaKinds.td

2031–2033

I wonder if we could just print out a simple access path here (eg x.a[3]). APValue has support for representing and pretty-printing such things already, and you could try passing arbitrary lvalue expressions to the constant evaluator to try to form them rather than special-casing a few things in getMemoryLocationImpl.

clang/lib/Sema/SemaChecking.cpp

12782–12783

Per the above, I think this can be replaced by calling EvaluateAsLValue on the expression.

[riccibruno]

clang/include/clang/Basic/DiagnosticSemaKinds.td

2031–2033

Ah yes perhaps this can work. I will take a look at that. Thanks for the pointer!

clang/include/clang/Basic/DiagnosticSemaKinds.td

def note_uninit_fixit_remove_cond : Note<
   "remove the %select{'%1' if its condition|condition if it}0 "
   "is always %select{false|true}2">;
 def err_init_incomplete_type : Error<"initialization of incomplete type %0">;
 def err_list_init_in_parens : Error<
   "cannot initialize %select{non-class|reference}0 type %1 with a "
   "parenthesized initializer list">;
 
 def warn_unsequenced_mod_mod : Warning<
-  "multiple unsequenced modifications to %0">, InGroup<Unsequenced>;
+  "multiple unsequenced modifications to "
+  "%select{|member |static member }0%2"
+  "%select{| of %3| of struct %4| of union %4| of class %4"
+  "| of structured binding %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 |static member }0%2"
+  "%select{| of %3| of struct %4| of union %4| of class %4"
+  "| of structured binding %3}1">, InGroup<Unsequenced>;
 
 def select_initialized_entity_kind : TextSubstitution<
   "%select{copying variable|copying parameter|"
   "returning object|initializing statement expression result|"
   "throwing object|copying member subobject|copying array element|"
   "allocating object|copying temporary|initializing base subobject|"
   "initializing vector element|capturing value}0">;
 

clang/lib/Sema/SemaChecking.cpp

else if (auto Call = dyn_cast<CallExpr>(E))
       Exprs.append(Call->arg_begin(), Call->arg_end());
     else if (auto Message = dyn_cast<ObjCMessageExpr>(E))
       Exprs.append(Message->arg_begin(), Message->arg_end());
   } while (!Exprs.empty());
 }
 
 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<const ValueDecl *, 1> ObjectOrCXXThis;
+  /// If non-null, a field in a record type.
+  const ValueDecl *Field = nullptr;
+
+public:
+  struct CXXThisTag {};
+  MemoryLocation() = default;
+  MemoryLocation(const ValueDecl *Object, const ValueDecl *Field)
+      : ObjectOrCXXThis(Object, /*IsCXXThis=*/false), Field(Field) {}
+  MemoryLocation(CXXThisTag, const ValueDecl *Field)
+      : ObjectOrCXXThis(nullptr, /*IsCXXThis=*/true), Field(Field) {}
+  MemoryLocation(void *OpaqueObject, const ValueDecl *Field) : Field(Field) {
+    ObjectOrCXXThis.setFromOpaqueValue(OpaqueObject);
+  }
+
+  explicit operator bool() const { return getObject() || isCXXThis(); }
+  const ValueDecl *getObject() const { return ObjectOrCXXThis.getPointer(); }
+  bool isCXXThis() const { return ObjectOrCXXThis.getInt(); }
+  const 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<const ValueDecl *, 1>;
+  using SecondTy = const ValueDecl *;
+  using FirstInfo = llvm::DenseMapInfo<FirstTy>;
+  using SecondInfo = llvm::DenseMapInfo<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) {
+    unsigned H1 = FirstInfo::getHashValue(MemoryLoc.ObjectOrCXXThis);
+    unsigned H2 = SecondInfo::getHashValue(MemoryLoc.Field);
+    // Taken from Boost hash_combine. llvm::DenseMapInfo<std::pair>
+    // is significantly slower.
+    return H2 ^ (H1 + 0x9e3779b9 + (H2 << 6) + (H2 >> 2));
+  }
+
+  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 ConstEvaluatedExprVisitor<SequenceChecker> {
   using Base = ConstEvaluatedExprVisitor<SequenceChecker>;
 
   /// A tree of sequenced regions within an expression. Two regions are
   /// unsequenced if one is an ancestor or a descendent of the other. When we
   /// finish processing an expression with sequencing, such as a comma
   /// expression, we fold its tree nodes into its parent, since they are
   /// unsequenced with respect to nodes we will visit later.
private:
     unsigned representative(unsigned K) {
       if (Values[K].Merged)
         // Perform path compression as we go.
         return Values[K].Parent = representative(Values[K].Parent);
       return K;
     }
   };
 
-  /// An object for which we can track unsequenced uses.
-  using Object = const 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 a memory location which is not sequenced before
+    /// the value computation of the expression, such as n++.
     UK_ModAsSideEffect,
 
     UK_Count = UK_ModAsSideEffect + 1
   };
 
   /// Bundle together a sequencing region and the expression corresponding
   /// to a specific usage. One Usage is stored for each usage kind in UsageInfo.
   struct Usage {
     const Expr *UsageExpr;
     SequenceTree::Seq Seq;
 
     Usage() : UsageExpr(nullptr), Seq() {}
   };
 
   struct UsageInfo {
     Usage Uses[UK_Count];
 
-    /// Have we issued a diagnostic for this object already?
+    /// Have we issued a diagnostic for this memory location already?
     bool Diagnosed;
 
     UsageInfo() : Uses(), Diagnosed(false) {}
   };
-  using UsageInfoMap = llvm::SmallDenseMap<Object, UsageInfo, 16>;
+  using UsageInfoMap = llvm::SmallDenseMap<MemoryLocation, UsageInfo, 16>;
 
   Sema &SemaRef;
 
   /// Sequenced regions within the expression.
   SequenceTree Tree;
 
   /// Declaration modifications and references which we have seen.
   UsageInfoMap UsageMap;
 
   /// The region we are currently within.
   SequenceTree::Seq Region;
 
   /// 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.
   SmallVectorImpl<const Expr *> &WorkList;
 
   /// RAII object wrapping the visitation of a sequenced subexpression of an
   /// expression. At the end of this process, the side-effects of the evaluation
   /// become sequenced with respect to the value computation of the result, so
   /// we downgrade any UK_ModAsSideEffect within the evaluation to
   /// UK_ModAsValue.
   struct SequencedSubexpression {
     SequencedSubexpression(SequenceChecker &Self)
       : Self(Self), OldModAsSideEffect(Self.ModAsSideEffect) {
       Self.ModAsSideEffect = &ModAsSideEffect;
     }
 
     ~SequencedSubexpression() {
-      for (const std::pair<Object, Usage> &M : llvm::reverse(ModAsSideEffect)) {
+      for (const 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).
         UsageInfo &UI = Self.UsageMap[M.first];
         auto &SideEffectUsage = UI.Uses[UK_ModAsSideEffect];
         Self.addUsage(M.first, UI, SideEffectUsage.UsageExpr, UK_ModAsValue);
         SideEffectUsage = M.second;
       }
       Self.ModAsSideEffect = OldModAsSideEffect;
     }
 
     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
   /// choose to evaluate as a constant. If any subexpression is evaluated and
   /// found to be non-constant, this allows us to suppress the evaluation of
   /// the outer expression.
   class EvaluationTracker {
   public:
public:
     }
 
   private:
     SequenceChecker &Self;
     EvaluationTracker *Prev;
     bool EvalOK = true;
   } *EvalTracker = nullptr;
 
-  /// Find the object which is produced by the specified expression,
-  /// if any.
-  Object getObject(const Expr *E, bool Mod) const {
+  /// Find the memory location which is produced by the specified
+  /// expression \p E if any.
+  static MemoryLocation getMemoryLocation(const Expr *E, bool Mod) {
+    return getMemoryLocationImpl(E, Mod, /*RefsSeenPtr=*/nullptr);
+  }
+
+  /// Implementation of \p getMemoryLocation.
+  /// \p RefsSeenPtr is used to avoid reference cycles. When such a cycle
+  /// is possible we check first if \p RefsSeenPtr is non-null. If it is
+  /// non-null we use the pointed \p SmallPtrSet and if null we create one on
+  /// the stack. This allow us to avoid creating the \p SmallPtrSet when we
+  /// don't strictly have to.
+  //
+  // FIXME: Is this duplicating some code which already exists somewhere else ?
+  // If not, should this be moved somewhere where it can be reused ?
+  static MemoryLocation
+  getMemoryLocationImpl(const Expr *E, bool Mod,
+                        llvm::SmallPtrSetImpl<const VarDecl *> *RefsSeenPtr) {
     E = E->IgnoreParenCasts();
-    if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(E)) {
+    if (const auto *UO = dyn_cast<UnaryOperator>(E)) {
       if (Mod && (UO->getOpcode() == UO_PreInc || UO->getOpcode() == UO_PreDec))
-        return getObject(UO->getSubExpr(), Mod);
-    } else if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
+        return getMemoryLocationImpl(UO->getSubExpr(), Mod, RefsSeenPtr);
+    }
+
+    else if (const auto *BO = dyn_cast<BinaryOperator>(E)) {
       if (BO->getOpcode() == BO_Comma)
-        return getObject(BO->getRHS(), Mod);
+        return getMemoryLocationImpl(BO->getRHS(), Mod, RefsSeenPtr);
       if (Mod && BO->isAssignmentOp())
-        return getObject(BO->getLHS(), Mod);
-    } else if (const 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 (const 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, RefsSeenPtr);
+    }
+
+    else if (const auto *ME = dyn_cast<MemberExpr>(E)) {
+      if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
+        return MemoryLocation(
+            /*Base=*/getMemoryLocationImpl(ME->getBase(), Mod, RefsSeenPtr)
+                .getOpaqueObject(),
+            /*Field=*/FD);
+
+      if (const auto *VD = dyn_cast<VarDecl>(ME->getMemberDecl())) {
+        if (!RefsSeenPtr) {
+          llvm::SmallPtrSet<const VarDecl *, 1> RefsSeen;
+          return maybeLookThroughRefs(VD, Mod, RefsSeen);
+        } else {
+          return maybeLookThroughRefs(VD, Mod, *RefsSeenPtr);
+        }
+      }
+    }
+
+    else if (const auto *CXXTE = dyn_cast<CXXThisExpr>(E)) {
+      return MemoryLocation(MemoryLocation::CXXThisTag(), /*Field=*/nullptr);
     }
 
-  /// Note that an object \p O was modified or used by an expression
-  /// \p UsageExpr with usage kind \p UK. \p UI is the \p UsageInfo for
-  /// the object \p O as obtained via the \p UsageMap.
-  void addUsage(Object O, UsageInfo &UI, const Expr *UsageExpr, UsageKind UK) {
+    else if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+      const ValueDecl *ReferencedDecl = DRE->getDecl();
+      if (const auto *VD = dyn_cast<VarDecl>(ReferencedDecl)) {
+        if (!RefsSeenPtr) {
+          llvm::SmallPtrSet<const VarDecl *, 1> RefsSeen;
+          return maybeLookThroughRefs(VD, Mod, RefsSeen);
+        } else {
+          return maybeLookThroughRefs(VD, Mod, *RefsSeenPtr);
+        }
+      }
+
+      if (const auto *BD = dyn_cast<BindingDecl>(ReferencedDecl)) {
+        MemoryLocation MemoryLoc =
+            getMemoryLocationImpl(BD->getBinding(), Mod, RefsSeenPtr);
+        // If we found a memory location, return it. Otherwise pretend that the
+        // binding is the memory location since this is better than not finding
+        // anything.
+        return MemoryLoc ? MemoryLoc : MemoryLocation(BD, /*Field=*/nullptr);
+      }
+    }
+    return MemoryLocation();
+  }
+
+  static MemoryLocation
+  maybeLookThroughRefs(const VarDecl *VD, bool Mod,
+                       llvm::SmallPtrSetImpl<const VarDecl *> &RefsSeen) {
+    MemoryLocation MemoryLoc;
+    // 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)
+        MemoryLoc = getMemoryLocationImpl(VD->getInit(), Mod, &RefsSeen);
+    }
+    // If we found a memory location, return it. Otherwise pretend that the
+    // given VarDecl is the memory location since this is better than not
+    // finding anything.
+    return MemoryLoc ? MemoryLoc : MemoryLocation(VD, /*Field=*/nullptr);
+  }
+
+  /// Note that a memory location \p MemoryLoc was modified or used by
+  /// an expression \p UsageExpr with usage kind \p UK. \p UI is
+  /// the \p UsageInfo for the memory location \p MemoryLoc as obtained
+  /// via the \p UsageMap.
+  void addUsage(MemoryLocation MemoryLoc, UsageInfo &UI, const Expr *UsageExpr,
+                UsageKind UK) {
+    assert(MemoryLoc && "addUsage requires a valid MemoryLocation!");
     // Get the old usage for the given object and usage kind.
     Usage &U = UI.Uses[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.
       U.UsageExpr = UsageExpr;
       U.Seq = Region;
     }
   }
 
-  /// Check whether a modification or use of an object \p O in an expression
-  /// \p UsageExpr conflicts with a prior usage of kind \p OtherKind. \p UI is
-  /// the \p UsageInfo for the object \p O as obtained via the \p UsageMap.
-  /// \p IsModMod is true when we are checking for a mod-mod unsequenced
-  /// usage and false we are checking for a mod-use unsequenced usage.
-  void checkUsage(Object O, UsageInfo &UI, const Expr *UsageExpr,
-                  UsageKind OtherKind, bool IsModMod) {
+  /// Check whether a modification or use of a memory location \p MemoryLoc in
+  /// an expression \p UsageExpr conflicts with a prior usage of kind
+  /// \p OtherKind. \p UI is the \p UsageInfo for the memory location
+  /// \p MemoryLoc as obtained via the \p UsageMap. \p IsModMod is true when we
+  /// are checking for a mod-mod unsequenced usage and false when we are
+  /// checking for a mod-use unsequenced usage.
+  void checkUsage(MemoryLocation MemoryLoc, UsageInfo &UI,
+                  const Expr *UsageExpr, UsageKind OtherKind, bool IsModMod) {
+    assert(MemoryLoc && "checkUsage requires a valid MemoryLocation!");
     if (UI.Diagnosed)
       return;
 
     const Usage &U = UI.Uses[OtherKind];
     if (!U.UsageExpr || !Tree.isUnsequenced(Region, U.Seq))
       return;
 
     const Expr *Mod = U.UsageExpr;
     const Expr *ModOrUse = UsageExpr;
     if (OtherKind == UK_Use)
       std::swap(Mod, ModOrUse);
 
+    enum MemberDiagKind { MDK_NotMember, MDK_Member, MDK_StaticMember };
+    enum OfDiagKind {
+      ODK_NoOf,
+      ODK_Of,
+      ODK_OfStruct,
+      ODK_OfUnion,
+      ODK_OfClass,
+      ODK_OfStructuredBinding
+    };
+    const ValueDecl *Object = MemoryLoc.getObject();
+    const ValueDecl *Field = MemoryLoc.getField();
+    const TagDecl *TD = nullptr;
+
+    MemberDiagKind MemberDiag =
+        (Field != nullptr) ? MDK_Member
+                           : (Object && isa<VarDecl>(Object) &&
+                              cast<VarDecl>(Object)->isStaticDataMember())
+                                 ? MDK_StaticMember
+                                 : MDK_NotMember;
+    OfDiagKind OfDiag = ODK_NoOf;
+    if (MemberDiag == MDK_Member) {
+      if (Object) {
+        if (isa<DecompositionDecl>(Object))
+          OfDiag = ODK_OfStructuredBinding;
+        else
+          OfDiag = ODK_Of;
+      }
+    } else if (MemberDiag == MDK_StaticMember) {
+      TD = dyn_cast<TagDecl>(cast<VarDecl>(Object)->getDeclContext());
+      if (TD) {
+        switch (TD->getTagKind()) {
+        case TTK_Struct:
+          OfDiag = ODK_OfStruct;
+          break;
+        case TTK_Class:
+          OfDiag = ODK_OfClass;
+          break;
+        case TTK_Union:
+          OfDiag = ODK_OfUnion;
+          break;
+        default:
+          break;
+        }
+      }
+    }
+
+    const ValueDecl *FirstDiagElement =
+        MemberDiag == MDK_Member ? Field : Object;
+    const ValueDecl *SecondDiagElement =
+        MemberDiag == MDK_Member ? Object : nullptr;
+
     SemaRef.DiagRuntimeBehavior(
         Mod->getExprLoc(), {Mod, ModOrUse},
         SemaRef.PDiag(IsModMod ? diag::warn_unsequenced_mod_mod
                                : diag::warn_unsequenced_mod_use)
-            << O << SourceRange(ModOrUse->getExprLoc()));
+            << static_cast<unsigned>(MemberDiag)
+            << static_cast<unsigned>(OfDiag) << FirstDiagElement
+            << SecondDiagElement << TD << SourceRange(ModOrUse->getExprLoc()));
+
     UI.Diagnosed = true;
   }
 
   // A note on note{Pre, Post}{Use, Mod}:
   //
   // (It helps to follow the algorithm with an expression such as
   //  "((++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.
   //
   // We then visit the children of the current node. After that notePostUse or
   // notePostMod is called. These will 1) detect an unsequenced modification
   // as side effect (as in "k++ + k") and 2) add a new usage with the
   // appropriate usage kind.
   //
   // We also have to be careful that some operation sequences modification as
   // side effect as well (for example: || or ,). To account for this we wrap
   // the visitation of such a sub-expression (for example: the LHS of || or ,)
   // with SequencedSubexpression. SequencedSubexpression is an RAII object
   // which record usages which are modifications as side effect, and then
   // downgrade them (or more accurately restore the previous usage which was a
   // modification as side effect) when exiting the scope of the sequenced
   // subexpression.
 
-  void notePreUse(Object O, const Expr *UseExpr) {
-    UsageInfo &UI = UsageMap[O];
+  void notePreUse(MemoryLocation MemoryLoc, const Expr *UseExpr) {
+    assert(MemoryLoc && "notePreUse requires a valid memory location!");
+    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, const Expr *UseExpr) {
-    UsageInfo &UI = UsageMap[O];
-    checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_ModAsSideEffect,
+  void notePostUse(MemoryLocation MemoryLoc, const Expr *UseExpr) {
+    assert(MemoryLoc && "notePostUse requires a valid memory location!");
+    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, const Expr *ModExpr) {
-    UsageInfo &UI = UsageMap[O];
+  void notePreMod(MemoryLocation MemoryLoc, const Expr *ModExpr) {
+    assert(MemoryLoc && "notePreMod requires a valid memory location!");
+    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, const Expr *ModExpr, UsageKind UK) {
-    UsageInfo &UI = UsageMap[O];
-    checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsSideEffect,
+  void notePostMod(MemoryLocation MemoryLoc, const Expr *ModExpr,
+                   UsageKind UK) {
+    assert(MemoryLoc && "notePostMod requires a valid memory location!");
+    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:
   SequenceChecker(Sema &S, const Expr *E,
                   SmallVectorImpl<const Expr *> &WorkList)
       : Base(S.Context), SemaRef(S), Region(Tree.root()), WorkList(WorkList) {
     Visit(E);
     // Silence a -Wunused-private-field since WorkList is now unused.
     // TODO: Evaluate if it can be used, and if not remove it.
     (void)this->WorkList;
   }
 
   void VisitStmt(const Stmt *S) {
     // Skip all statements which aren't expressions for now.
   }
 
   void VisitExpr(const Expr *E) {
     // By default, just recurse to evaluated subexpressions.
     Base::VisitStmt(E);
   }
 
   void VisitCastExpr(const CastExpr *E) {
-    Object O = Object();
+    MemoryLocation MemoryLoc{};
     if (E->getCastKind() == CK_LValueToRValue)
-      O = getObject(E->getSubExpr(), false);
+      MemoryLoc = getMemoryLocation(E->getSubExpr(), /*Mod=*/false);
+
+    if (MemoryLoc)
+      notePreUse(MemoryLoc, E);
 
-    if (O)
-      notePreUse(O, E);
     VisitExpr(E);
-    if (O)
-      notePostUse(O, E);
+
+    if (MemoryLoc)
+      notePostUse(MemoryLoc, E);
   }
 
   void VisitSequencedExpressions(const Expr *SequencedBefore,
                                  const Expr *SequencedAfter) {
     SequenceTree::Seq BeforeRegion = Tree.allocate(Region);
     SequenceTree::Seq AfterRegion = Tree.allocate(Region);
     SequenceTree::Seq OldRegion = Region;
 
     {
       SequencedSubexpression SeqBefore(*this);
       Region = BeforeRegion;
       Visit(SequencedBefore);
void VisitBinAssign(const BinaryOperator *BO) {
     SequenceTree::Seq OldRegion = Region;
 
     // C++11 [expr.ass]p1:
     //  [...] the assignment is sequenced after the value computation
     //  of the right and left operands, [...]
     //
     // so check it before inspecting the operands and update the
     // map afterwards.
-    Object O = getObject(BO->getLHS(), /*Mod=*/true);
-    if (O)
-      notePreMod(O, BO);
+    MemoryLocation MemoryLoc = getMemoryLocation(BO->getLHS(), /*Mod=*/true);
+    if (MemoryLoc)
+      notePreMod(MemoryLoc, BO);
 
     if (SemaRef.getLangOpts().CPlusPlus17) {
       // C++17 [expr.ass]p1:
       //  [...] The right operand is sequenced before the left operand. [...]
       {
         SequencedSubexpression SeqBefore(*this);
         Region = RHSRegion;
         Visit(BO->getRHS());
       }
 
       Region = LHSRegion;
       Visit(BO->getLHS());
 
-      if (O && isa<CompoundAssignOperator>(BO))
-        notePostUse(O, BO);
+      if (MemoryLoc && isa<CompoundAssignOperator>(BO))
+        notePostUse(MemoryLoc, BO);
 
     } else {
       // C++11 does not specify any sequencing between the LHS and RHS.
       Region = LHSRegion;
       Visit(BO->getLHS());
 
-      if (O && isa<CompoundAssignOperator>(BO))
-        notePostUse(O, BO);
+      if (MemoryLoc && isa<CompoundAssignOperator>(BO))
+        notePostUse(MemoryLoc, BO);
 
       Region = RHSRegion;
       Visit(BO->getRHS());
     }
 
     // C++11 [expr.ass]p1:
     //  the assignment is sequenced [...] before the value computation of the
     //  assignment expression.
     // C11 6.5.16/3 has no such rule.
     Region = OldRegion;
-    if (O)
-      notePostMod(O, BO,
+    if (MemoryLoc)
+      notePostMod(MemoryLoc, BO,
                   SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
                                                   : UK_ModAsSideEffect);
     if (SemaRef.getLangOpts().CPlusPlus17) {
       Tree.merge(RHSRegion);
       Tree.merge(LHSRegion);
     }
   }
 
   void VisitCompoundAssignOperator(const CompoundAssignOperator *CAO) {
     VisitBinAssign(CAO);
   }
 
   void VisitUnaryPreInc(const UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }
   void VisitUnaryPreDec(const UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }
   void VisitUnaryPreIncDec(const 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,
+    notePostMod(MemoryLoc, UO,
                 SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
                                                 : UK_ModAsSideEffect);
   }
 
   void VisitUnaryPostInc(const UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }
   void VisitUnaryPostDec(const UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }
   void VisitUnaryPostIncDec(const 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);
   }
 
   void VisitBinLOr(const BinaryOperator *BO) {
     // C++11 [expr.log.or]p2:
     //  If the second expression is evaluated, every value computation and
     //  side effect associated with the first expression is sequenced before
     //  every value computation and side effect associated with the
     //  second expression.

clang/test/SemaCXX/warn-unsequenced.cpp

struct S1 {
   unsigned bf1 : 2;
   unsigned bf2 : 2;
   unsigned a;
   unsigned b;
   static unsigned x;
   void member_f(S1 &s);
 };
 
+class SomeClass { public: static int x; };
+union SomeUnion { public: static int x; };
+
 void S1::member_f(S1 &s) {
-  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
-             // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
-  a + ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-           // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to member 'a'}}
+             // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a'}}
+  a + ++a; // cxx11-warning {{unsequenced modification and access to member 'a'}}
+           // cxx17-warning@-1 {{unsequenced modification and access to member 'a'}}
   ++a + ++b; // no-warning
   a + ++b; // no-warning
 
-  // TODO: Warn here.
-  ++s.a + ++s.a; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.a + ++s.a; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.a + ++s.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 's'}}
+  s.a + ++s.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 's'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 's'}}
   ++s.a + ++s.b; // no-warning
   s.a + ++s.b; // no-warning
 
   ++a + ++s.a; // no-warning
   a + ++s.a; // no-warning
   ++a + ++s.b; // no-warning
   a + ++s.b; // no-warning
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to 'bf1'}}
-                 // cxx17-warning@-1 {{multiple unsequenced modifications to 'bf1'}}
-  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to 'bf1'}}
-               // cxx17-warning@-1 {{unsequenced modification and access to 'bf1'}}
+  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1'}}
+  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1'}}
   ++bf1 + ++bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   bf1 + ++bf2; // no-warning TODO {{unsequenced modification and access to}}
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++s.bf1 + ++s.bf1; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.bf1 + ++s.bf1; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.bf1 + ++s.bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1' of 's'}}
+                     // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1' of 's'}}
+  s.bf1 + ++s.bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1' of 's'}}
+                   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1' of 's'}}
   ++s.bf1 + ++s.bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   s.bf1 + ++s.bf2; // no-warning TODO {{unsequenced modification and access to}}
 
   ++bf1 + ++s.bf1; // no-warning
   bf1 + ++s.bf1; // no-warning
   ++bf1 + ++s.bf2; // no-warning
   bf1 + ++s.bf2; // no-warning
 
   struct Der : S1 {};
   Der d;
   Der &d_ref = d;
   S1 &s1_ref = d_ref;
 
-  ++s1_ref.a + ++d_ref.a; // no-warning TODO {{multiple unsequenced modifications to member 'a' of 'd'}}
-  ++s1_ref.a + d_ref.a; // no-warning TODO {{unsequenced modification and access to member 'a' of 'd'}}
+  ++s1_ref.a + ++d_ref.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 'd'}}
+                          // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 'd'}}
+  ++s1_ref.a + d_ref.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 'd'}}
+                        // cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 'd'}}
   ++s1_ref.a + ++d_ref.b; // no-warning
   ++s1_ref.a + d_ref.b; // no-warning
 
-  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
-             // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-  ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
-           // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
-  ++s.x + x; // no-warning TODO {{unsequenced modification and access to static member 'x' of 'S1'}}
-  ++this->x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
-                 // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
-  ++d_ref.x + ++S1::x; // no-warning TODO {{unsequenced modification and access to static member 'x' of 'S1'}}
+  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to static member 'x' of struct 'S1'}}
+             // cxx17-warning@-1 {{multiple unsequenced modifications to static member 'x' of struct 'S1'}}
+  ++x + x; // cxx11-warning {{unsequenced modification and access to static member 'x' of struct 'S1'}}
+           // cxx17-warning@-1 {{unsequenced modification and access to static member 'x' of struct 'S1'}}
+  ++s.x + x; // cxx11-warning {{unsequenced modification and access to static member 'x' of struct 'S1'}}
+             // cxx17-warning@-1 {{unsequenced modification and access to static member 'x' of struct 'S1'}}
+  ++this->x + x; // cxx11-warning {{unsequenced modification and access to static member 'x' of struct 'S1'}}
+                 // cxx17-warning@-1 {{unsequenced modification and access to static member 'x' of struct 'S1'}}
+  ++d_ref.x + S1::x; // cxx11-warning {{unsequenced modification and access to static member 'x' of struct 'S1'}}
+                     // cxx17-warning@-1 {{unsequenced modification and access to static member 'x' of struct 'S1'}}
+
+  SomeClass::x++ + SomeClass::x; // cxx11-warning {{unsequenced modification and access to static member 'x' of class 'SomeClass'}}
+                                 // cxx17-warning@-1 {{unsequenced modification and access to static member 'x' of class 'SomeClass'}}
+  SomeUnion::x++ + SomeUnion::x; // cxx11-warning {{unsequenced modification and access to static member 'x' of union 'SomeUnion'}}
+                                 // cxx17-warning@-1 {{unsequenced modification and access to static member 'x' of union 'SomeUnion'}}
+  SomeClass::x++ + SomeUnion::x++; // no-warning
 }
 
 struct S2 {
   union { unsigned x, y; };
   void f2();
 };
 
 void S2::f2() {
-  ++x + ++x; // no-warning TODO {{multiple unsequenced modifications to}}
-  x + ++x; // no-warning TODO {{unsequenced modification and access to}}
+  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to member 'x'}}
+             // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x'}}
+  x + ++x; // cxx11-warning {{unsequenced modification and access to member 'x'}}
+           // cxx17-warning@-1 {{unsequenced modification and access to member 'x'}}
   ++x + ++y; // no-warning
   x + ++y; // no-warning
 }
 
 void f2(S2 &s) {
-  ++s.x + ++s.x; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.x + ++s.x; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.x + ++s.x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of 's'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of 's'}}
+  s.x + ++s.x; // cxx11-warning {{unsequenced modification and access to member 'x' of 's'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of 's'}}
   ++s.x + ++s.y; // no-warning
   s.x + ++s.y; // no-warning
 }
 
 struct S3 {
   union {
     union {
       unsigned x;
     };
   };
   unsigned y;
   void f3();
 };
 
 void S3::f3() {
-  ++x + ++x; // no-warning TODO {{multiple unsequenced modifications to}}
-  x + ++x; // no-warning TODO {{unsequenced modification and access to}}
+  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to member 'x'}}
+             // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x'}}
+  x + ++x; // cxx11-warning {{unsequenced modification and access to member 'x'}}
+           // cxx17-warning@-1 {{unsequenced modification and access to member 'x'}}
   ++x + ++y; // no-warning
   x + ++y; // no-warning
 }
 
 void f3(S3 &s) {
-  ++s.x + ++s.x; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.x + ++s.x; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.x + ++s.x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of 's'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of 's'}}
+  s.x + ++s.x; // cxx11-warning {{unsequenced modification and access to member 'x' of 's'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of 's'}}
   ++s.x + ++s.y; // no-warning
   s.x + ++s.y; // no-warning
 }
 
 struct S4 : S3 {
   unsigned y;
   void f4();
 };
 
 void S4::f4() {
-  ++x + ++x; // no-warning TODO {{multiple unsequenced modifications to}}
-  x + ++x; // no-warning TODO {{unsequenced modification and access to}}
+  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to member 'x'}}
+             // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x'}}
+  x + ++x; // cxx11-warning {{unsequenced modification and access to member 'x'}}
+           // cxx17-warning@-1 {{unsequenced modification and access to member 'x'}}
   ++x + ++y; // no-warning
   x + ++y; // no-warning
   ++S3::y + ++y; // no-warning
   S3::y + ++y; // no-warning
 }
 
 void f4(S4 &s) {
-  ++s.x + ++s.x; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.x + ++s.x; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.x + ++s.x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of 's'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of 's'}}
+  s.x + ++s.x; // cxx11-warning {{unsequenced modification and access to member 'x' of 's'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of 's'}}
   ++s.x + ++s.y; // no-warning
   s.x + ++s.y; // no-warning
   ++s.S3::y + ++s.y; // no-warning
   s.S3::y + ++s.y; // no-warning
 }
 
 static union {
   unsigned Ux;
   unsigned Uy;
 };
 
 void f5() {
-  ++Ux + ++Ux; // no-warning TODO {{multiple unsequenced modifications to}}
-  Ux + ++Ux; // no-warning TODO {{unsequenced modification and access to}}
+  ++Ux + ++Ux; // cxx11-warning {{multiple unsequenced modifications to member 'Ux' of ''}}
+               // cxx17-warning@-1 {{multiple unsequenced modifications to member 'Ux' of ''}}
+  Ux + ++Ux; // cxx11-warning {{unsequenced modification and access to member 'Ux' of ''}}
+             // cxx17-warning@-1 {{unsequenced modification and access to member 'Ux' of ''}}
   ++Ux + ++Uy; // no-warning
   Ux + ++Uy; // no-warning
 }
 
 void f6() {
   struct S { unsigned x, y; } s;
-  ++s.x + ++s.x; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.x + ++s.x; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.x + ++s.x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of 's'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of 's'}}
+  s.x + ++s.x; // cxx11-warning {{unsequenced modification and access to member 'x' of 's'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of 's'}}
   ++s.x + ++s.y; // no-warning
   s.x + ++s.y; // no-warning
 
   struct { unsigned x, y; } t;
-  ++t.x + ++t.x; // no-warning TODO {{multiple unsequenced modifications to}}
-  t.x + ++t.x; // no-warning TODO {{unsequenced modification and access to}}
+  ++t.x + ++t.x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of 't'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of 't'}}
+  t.x + ++t.x; // cxx11-warning {{unsequenced modification and access to member 'x' of 't'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of 't'}}
   ++t.x + ++t.y; // no-warning
   t.x + ++t.y; // no-warning
 }
 
 } // namespace members
 
 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;
   int &c = b;
   int &ra1 = c;
   int &ra2 = b;
   int other;
 
-  ++ra1 + ++ra2; // no-warning TODO {{multiple unsequenced modifications to}}
-  ra1 + ++ra2; // no-warning TODO {{unsequenced modification and access to}}
+  ++ra1 + ++ra2; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
+  ra1 + ++ra2; // cxx11-warning {{unsequenced modification and access to 'a'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   ++ra1 + ++other; // no-warning
   ra1 + ++other; // no-warning
 
   // Make sure we handle reference cycles.
   int &ref_cycle = ref_cycle;
   ++ref_cycle + ++ref_cycle; // cxx11-warning {{multiple unsequenced modifications to 'ref_cycle'}}
                              // cxx17-warning@-1 {{multiple unsequenced modifications to 'ref_cycle'}}
   ref_cycle + ++ref_cycle; // cxx11-warning {{unsequenced modification and access to 'ref_cycle'}}

 
 } // namespace bindings
 template<> struct std::tuple_size<bindings::C> { enum { value = 2 }; };
 namespace bindings {
 void testa() {
   A a;
   {
     auto [x, y] = a;
-    ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
-               // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-    ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
-             // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
+    ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of structured binding '[x, y]'}}
+               // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of structured binding '[x, y]'}}
+    ++x + x; // cxx11-warning {{unsequenced modification and access to member 'x' of structured binding '[x, y]'}}
+             // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of structured binding '[x, y]'}}
     ++x + ++y; // no-warning
     ++x + y; // no-warning
     ++x + ++a.x; // no-warning
     ++x + a.x; // no-warning
   }
   {
     auto &[x, y] = a;
-    ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
-               // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-    ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
-             // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
+    ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of 'a'}}
+               // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of 'a'}}
+    ++x + x; // cxx11-warning {{unsequenced modification and access to member 'x' of 'a'}}
+             // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of 'a'}}
     ++x + ++y; // no-warning
     ++x + y; // no-warning
-    ++x + ++a.x; // no-warning TODO
-    ++x + a.x; // no-warning TODO
+    ++x + ++a.x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of 'a'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of 'a'}}
+    ++x + a.x; // cxx11-warning {{unsequenced modification and access to member 'x' of 'a'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of 'a'}}
   }
 }
+
 void testb() {
   B b;
   {
     auto [x, y] = b;
     ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
                // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
     ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
              // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
     ++x + ++y; // no-warning
     ++x + y; // no-warning
     ++x + ++b[0]; // no-warning
     ++x + b[0]; // no-warning
   }
   {
     auto &[x, y] = b;
     ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
                // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
     ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
              // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
     ++x + ++y; // no-warning
     ++x + y; // no-warning
+    // TODO: We don't handle these cases because we don't track memory locations
+    // through array subscript expressions for now.
     ++x + ++b[0]; // no-warning TODO
     ++x + b[0]; // no-warning TODO
   }
 }
 void testc() {
   C c;
   {
     auto [x, y] = c;
C c;
     ++x + ++y; // no-warning
     ++x + y; // no-warning
   }
 }
 void testd() {
   D d;
   {
     auto [x, y] = d;
-    ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
-               // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-    ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
-             // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
+    ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of structured binding '[x, y]'}}
+               // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of structured binding '[x, y]'}}
+    ++x + x; // cxx11-warning {{unsequenced modification and access to member 'x' of structured binding '[x, y]'}}
+             // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of structured binding '[x, y]'}}
     ++x + ++y; // no-warning
     ++x + y; // no-warning
     ++x + ++d.x; // no-warning
     ++x + d.x; // no-warning
   }
   {
     auto &[x, y] = d;
-    ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
-               // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-    ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
-             // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
+    ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of 'd'}}
+               // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of 'd'}}
+    ++x + x; // cxx11-warning {{unsequenced modification and access to member 'x' of 'd'}}
+             // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of 'd'}}
     ++x + ++y; // no-warning
     ++x + y; // no-warning
-    ++x + ++d.x; // no-warning TODO
-    ++x + d.x; // no-warning TODO
+    ++x + ++d.x; // cxx11-warning {{multiple unsequenced modifications to member 'x' of 'd'}}
+                 // cxx17-warning@-1 {{multiple unsequenced modifications to member 'x' of 'd'}}
+    ++x + d.x; // cxx11-warning {{unsequenced modification and access to member 'x' of 'd'}}
+               // cxx17-warning@-1 {{unsequenced modification and access to member 'x' of 'd'}}
   }
 }
 } // namespace bindings
 
 namespace templates {
 
 template <typename T>
 struct Bar {

 
 template <typename T> int var = sizeof(T);
 void test_var() {
   var<int>++ + var<int>++; // cxx11-warning {{multiple unsequenced modifications to 'var<int>'}}
                            // cxx17-warning@-1 {{multiple unsequenced modifications to 'var<int>'}}
   var<int>++ + var<int>; // cxx11-warning {{unsequenced modification and access to 'var<int>'}}
                          // cxx17-warning@-1 {{unsequenced modification and access to 'var<int>'}}
   int &r = var<int>;
-  r++ + var<int>++; // no-warning TODO {{multiple unsequenced modifications to 'var<int>'}}
+  r++ + var<int>++; // cxx11-warning {{multiple unsequenced modifications to 'var<int>'}}
+                    // cxx17-warning@-1 {{multiple unsequenced modifications to 'var<int>'}}
   r++ + var<long>++; // no-warning
 }
 
 } // namespace templates