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