[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

12361 ↗

(On Diff #216153)

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

[xbolva00]

clang/lib/Sema/SemaChecking.cpp

12631 ↗

(On Diff #216153)

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 ↗

(On Diff #234627)

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 ↗

(On Diff #234627)

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

[riccibruno]

clang/include/clang/Basic/DiagnosticSemaKinds.td

2031–2033 ↗

(On Diff #234627)

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

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">;
 

include/clang/Sema/Sema.h

 
 public:
   void CheckFloatComparison(SourceLocation Loc, Expr *LHS, Expr *RHS);
 
 private:
   void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());
   void CheckBoolLikeConversion(Expr *E, SourceLocation CC);
   void CheckForIntOverflow(Expr *E);
-  void CheckUnsequencedOperations(Expr *E);
+  void CheckUnsequencedOperations(const Expr *E);
 
   /// Perform semantic checks on a completed expression. This will either
   /// be a full-expression or a default argument expression.
   void CheckCompletedExpr(Expr *E, SourceLocation CheckLoc = SourceLocation(),
                           bool IsConstexpr = false);
 
   void CheckBitFieldInitialization(SourceLocation InitLoc, FieldDecl *Field,
                                    Expr *Init);

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.
-class SequenceChecker : public EvaluatedExprVisitor<SequenceChecker> {
-  using Base = EvaluatedExprVisitor<SequenceChecker>;
+/// 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.
   class SequenceTree {
     struct Value {
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 = 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
   };
 
   /// 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 {
-    Expr *UsageExpr = nullptr;
+    const Expr *UsageExpr = nullptr;
     SequenceTree::Seq Seq;
     Usage() = default;
-    Usage(Expr *UsageExpr, SequenceTree::Seq Seq)
+    Usage(const Expr *UsageExpr, SequenceTree::Seq Seq)
         : UsageExpr(UsageExpr), Seq(Seq) {}
   };
 
   class UsageInfo {
     /// The expressions corresponding to each usage kind.
-    Expr *UsageExprs[UK_Count]{};
+    const Expr *UsageExprs[UK_Count]{};
 
     /// 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:
     Usage getUsage(UsageKind UK) const {
       assert(UK < UK_Count && "Invalid UsageKind!");
       return Usage{UsageExprs[UK], Seqs[UK]};
     }
     void setUsage(UsageKind UK, Usage U) {
       assert(UK < UK_Count && "Invalid UsageKind!");
       UsageExprs[UK] = U.UsageExpr;
       Seqs[UK] = U.Seq;
     }
     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;
 
   /// 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<Expr *> &WorkList;
+  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 (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).
         UsageInfo &UI = Self.UsageMap[M.first];
         Usage SideEffectUsage = UI.getUsage(UK_ModAsSideEffect);
         Self.addUsage(M.first, UI, SideEffectUsage.UsageExpr, UK_ModAsValue);
         UI.setUsage(UK_ModAsSideEffect, 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,
+  /// Find the memory location which is produced by the specified expression,
   /// if any.
-  Object getObject(Expr *E, bool Mod) const {
+  static MemoryLocation getMemoryLocation(const Expr *E, bool Mod) {
+    return getMemoryLocationImpl(E, Mod, /*RefsSeenPtr=*/nullptr);
+  }
+
+  /// Implementation of getMemoryLocation().
+  /// \param RefsSeenPtr is used to avoid reference cycles. When such a cycle
+  /// is possible we check first if \param RefsSeenPtr is non-null. If it is
+  /// non-null we use the pointed SmallPtrSet and if null we create one on the
+  /// stack. This allow us to avoid creating the SmallPtrSet when we don't
+  /// strictly have to.
+  static MemoryLocation
+  getMemoryLocationImpl(const Expr *E, bool Mod,
+                        llvm::SmallPtrSetImpl<const VarDecl *> *RefsSeenPtr) {
     E = E->IgnoreParenCasts();
-    if (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 (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 (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, RefsSeenPtr);
     }
 
-  /// 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.
+    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);
+    }
+
+    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 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, const Expr *UsageExpr,
+                UsageKind UK) {
+    assert(MemoryLoc && "addUsage requires a valid MemoryLocation!");
+    // 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,
+                  const Expr *UsageExpr, UsageKind OtherKind, bool IsModMod) {
+    assert(MemoryLoc && "checkUsage requires a valid MemoryLocation!");
     if (UI.alreadyDiagnosed())
       return;
 
     Usage U = UI.getUsage(OtherKind);
     if (!U.UsageExpr || !Tree.isUnsequenced(Region, U.Seq))
       return;
 
-    Expr *Mod = U.UsageExpr;
-    Expr *ModOrUse = UsageExpr;
+    const Expr *Mod = U.UsageExpr;
+    const Expr *ModOrUse = UsageExpr;
     if (OtherKind == UK_Use)
       std::swap(Mod, ModOrUse);
 
-    SemaRef.Diag(Mod->getExprLoc(),
-                 IsModMod ? diag::warn_unsequenced_mod_mod
+    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.Diag(Mod->getExprLoc(), 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.markDiagnosed();
   }
 
   // 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. 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
   // 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, 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, 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, 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, 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, Expr *E, SmallVectorImpl<Expr *> &WorkList)
+  SequenceChecker(Sema &S, const Expr *E,
+                  SmallVectorImpl<const Expr *> &WorkList)
       : Base(S.Context), SemaRef(S), Region(Tree.root()), WorkList(WorkList) {
     Visit(E);
   }
 
-  void VisitStmt(Stmt *S) {
+  void VisitStmt(const Stmt *S) {
     // Skip all statements which aren't expressions for now.
   }
 
-  void VisitExpr(Expr *E) {
+  void VisitExpr(const Expr *E) {
     // By default, just recurse to evaluated subexpressions.
     Base::VisitStmt(E);
   }
 
-  void VisitCastExpr(CastExpr *E) {
-    Object O = Object();
+  void VisitCastExpr(const CastExpr *E) {
+    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) {
+  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);
     }
 
     Region = AfterRegion;
     Visit(SequencedAfter);
 
     Region = OldRegion;
 
     Tree.merge(BeforeRegion);
     Tree.merge(AfterRegion);
   }
 
-  void VisitArraySubscriptExpr(ArraySubscriptExpr *ASE) {
+  void VisitArraySubscriptExpr(const ArraySubscriptExpr *ASE) {
     // C++17 [expr.sub]p1:
     //   The expression E1[E2] is identical (by definition) to *((E1)+(E2)). The
     //   expression E1 is sequenced before the expression E2.
     if (SemaRef.getLangOpts().CPlusPlus17)
       VisitSequencedExpressions(ASE->getLHS(), ASE->getRHS());
     else
       Base::VisitStmt(ASE);
   }
 
-  void VisitBinComma(BinaryOperator *BO) {
+  void VisitBinComma(const BinaryOperator *BO) {
     // C++11 [expr.comma]p1:
     //   Every value computation and side effect associated with the left
     //   expression is sequenced before every value computation and side
     //   effect associated with the right expression.
     VisitSequencedExpressions(BO->getLHS(), BO->getRHS());
   }
 
-  void VisitBinAssign(BinaryOperator *BO) {
+  void VisitBinAssign(const BinaryOperator *BO) {
     // 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
     //   only once.
     //
     // 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());
 
     // 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.
-    notePostMod(O, BO, SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
-                                                       : UK_ModAsSideEffect);
+    notePostMod(MemoryLoc, BO,
+                SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
   }
 
-  void VisitCompoundAssignOperator(CompoundAssignOperator *CAO) {
+  void VisitCompoundAssignOperator(const CompoundAssignOperator *CAO) {
     VisitBinAssign(CAO);
   }
 
-  void VisitUnaryPreInc(UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }
-  void VisitUnaryPreDec(UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }
-  void VisitUnaryPreIncDec(UnaryOperator *UO) {
-    Object O = getObject(UO->getSubExpr(), true);
-    if (!O)
+  void VisitUnaryPreInc(const UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }
+  void VisitUnaryPreDec(const UnaryOperator *UO) { VisitUnaryPreIncDec(UO); }
+  void VisitUnaryPreIncDec(const UnaryOperator *UO) {
+    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
   }
 
-  void VisitUnaryPostInc(UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }
-  void VisitUnaryPostDec(UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }
-  void VisitUnaryPostIncDec(UnaryOperator *UO) {
-    Object O = getObject(UO->getSubExpr(), true);
-    if (!O)
+  void VisitUnaryPostInc(const UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }
+  void VisitUnaryPostDec(const UnaryOperator *UO) { VisitUnaryPostIncDec(UO); }
+  void VisitUnaryPostIncDec(const UnaryOperator *UO) {
+    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.
-  void VisitBinLOr(BinaryOperator *BO) {
+  void VisitBinLOr(const BinaryOperator *BO) {
     // The side-effects of the LHS of an '&&' are sequenced before the
     // value computation of the RHS, and hence before the value computation
     // of the '&&' itself, unless the LHS evaluates to zero. We treat them
     // as if they were unconditionally sequenced.
     EvaluationTracker Eval(*this);
     {
       SequencedSubexpression Sequenced(*this);
       Visit(BO->getLHS());
     }
 
     bool Result;
     if (Eval.evaluate(BO->getLHS(), Result)) {
       if (!Result)
         Visit(BO->getRHS());
     } else {
       // Check for unsequenced operations in the RHS, treating it as an
       // entirely separate evaluation.
       //
       // FIXME: If there are operations in the RHS which are unsequenced
       // with respect to operations outside the RHS, and those operations
       // are unconditionally evaluated, diagnose them.
       WorkList.push_back(BO->getRHS());
     }
   }
-  void VisitBinLAnd(BinaryOperator *BO) {
+  void VisitBinLAnd(const BinaryOperator *BO) {
     EvaluationTracker Eval(*this);
     {
       SequencedSubexpression Sequenced(*this);
       Visit(BO->getLHS());
     }
 
     bool Result;
     if (Eval.evaluate(BO->getLHS(), Result)) {
       if (Result)
         Visit(BO->getRHS());
     } else {
       WorkList.push_back(BO->getRHS());
     }
   }
 
   // Only visit the condition, unless we can be sure which subexpression will
   // be chosen.
-  void VisitAbstractConditionalOperator(AbstractConditionalOperator *CO) {
+  void VisitAbstractConditionalOperator(const AbstractConditionalOperator *CO) {
     EvaluationTracker Eval(*this);
     {
       SequencedSubexpression Sequenced(*this);
       Visit(CO->getCond());
     }
 
     bool Result;
     if (Eval.evaluate(CO->getCond(), Result))
       Visit(Result ? CO->getTrueExpr() : CO->getFalseExpr());
     else {
       WorkList.push_back(CO->getTrueExpr());
       WorkList.push_back(CO->getFalseExpr());
     }
   }
 
-  void VisitCallExpr(CallExpr *CE) {
+  void VisitCallExpr(const CallExpr *CE) {
     // C++11 [intro.execution]p15:
     //   When calling a function [...], every value computation and side effect
     //   associated with any argument expression, or with the postfix expression
     //   designating the called function, is sequenced before execution of every
     //   expression or statement in the body of the function [and thus before
     //   the value computation of its result].
     SequencedSubexpression Sequenced(*this);
     Base::VisitCallExpr(CE);
 
     // FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
   }
 
-  void VisitCXXConstructExpr(CXXConstructExpr *CCE) {
+  void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
     // This is a call, so all subexpressions are sequenced before the result.
     SequencedSubexpression Sequenced(*this);
 
     if (!CCE->isListInitialization())
       return VisitExpr(CCE);
 
     // In C++11, list initializations are sequenced.
     SmallVector<SequenceTree::Seq, 32> Elts;
     SequenceTree::Seq Parent = Region;
-    for (CXXConstructExpr::arg_iterator I = CCE->arg_begin(),
+    for (CXXConstructExpr::const_arg_iterator I = CCE->arg_begin(),
                                               E = CCE->arg_end();
          I != E; ++I) {
       Region = Tree.allocate(Parent);
       Elts.push_back(Region);
       Visit(*I);
     }
 
     // Forget that the initializers are sequenced.
     Region = Parent;
     for (unsigned I = 0; I < Elts.size(); ++I)
       Tree.merge(Elts[I]);
   }
 
-  void VisitInitListExpr(InitListExpr *ILE) {
+  void VisitInitListExpr(const InitListExpr *ILE) {
     if (!SemaRef.getLangOpts().CPlusPlus11)
       return VisitExpr(ILE);
 
     // In C++11, list initializations are sequenced.
     SmallVector<SequenceTree::Seq, 32> Elts;
     SequenceTree::Seq Parent = Region;
     for (unsigned I = 0; I < ILE->getNumInits(); ++I) {
-      Expr *E = ILE->getInit(I);
+      const Expr *E = ILE->getInit(I);
       if (!E) continue;
       Region = Tree.allocate(Parent);
       Elts.push_back(Region);
       Visit(E);
     }
 
     // Forget that the initializers are sequenced.
     Region = Parent;
     for (unsigned I = 0; I < Elts.size(); ++I)
       Tree.merge(Elts[I]);
   }
 };
 
 } // namespace
 
-void Sema::CheckUnsequencedOperations(Expr *E) {
-  SmallVector<Expr *, 8> WorkList;
+void Sema::CheckUnsequencedOperations(const Expr *E) {
+  SmallVector<const Expr *, 8> WorkList;
   WorkList.push_back(E);
   while (!WorkList.empty()) {
-    Expr *Item = WorkList.pop_back_val();
+    const Expr *Item = WorkList.pop_back_val();
     SequenceChecker(*this, Item, WorkList);
   }
 }
 
 void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc,
                               bool IsConstexpr) {
   CheckImplicitConversions(E, CheckLoc);
   if (!E->isInstantiationDependent())

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