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