NoQ updated this revision to Diff 155091.
NoQ added a comment.
Address my own comments.
https://reviews.llvm.org/D48681
Files:
include/clang/Analysis/CFG.h
include/clang/Analysis/ConstructionContext.h
lib/Analysis/CFG.cpp
lib/Analysis/ConstructionContext.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/cfg-rich-constructors.cpp
test/Analysis/cfg-rich-constructors.mm
Index: test/Analysis/cfg-rich-constructors.mm
===================================================================
--- test/Analysis/cfg-rich-constructors.mm
+++ test/Analysis/cfg-rich-constructors.mm
@@ -18,21 +18,21 @@
-(D) bar;
@end
-// FIXME: Find construction context for the argument.
// CHECK: void passArgumentIntoMessage(E *e)
// CHECK: 1: e
// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, LValueToRValue, E *)
-// CXX11-NEXT: 3: D() (CXXConstructExpr, [B1.4], [B1.6], class D)
+// CXX11-ELIDE-NEXT: 3: D() (CXXConstructExpr, [B1.4], [B1.6], [B1.7], class D)
+// CXX11-NOELIDE-NEXT: 3: D() (CXXConstructExpr, [B1.4], [B1.6], class D)
// CXX11-NEXT: 4: [B1.3] (BindTemporary)
// CXX11-NEXT: 5: [B1.4] (ImplicitCastExpr, NoOp, const class D)
// CXX11-NEXT: 6: [B1.5]
-// CXX11-NEXT: 7: [B1.6] (CXXConstructExpr, class D)
+// CXX11-NEXT: 7: [B1.6] (CXXConstructExpr, [B1.8], [B1.9]+0, class D)
// CXX11-NEXT: 8: [B1.7] (BindTemporary)
// Double brackets trigger FileCheck variables, escape.
// CXX11-NEXT: 9: {{\[}}[B1.2] foo:[B1.8]]
// CXX11-NEXT: 10: ~D() (Temporary object destructor)
// CXX11-NEXT: 11: ~D() (Temporary object destructor)
-// CXX17-NEXT: 3: D() (CXXConstructExpr, class D)
+// CXX17-NEXT: 3: D() (CXXConstructExpr, [B1.4], [B1.5]+0, class D)
// CXX17-NEXT: 4: [B1.3] (BindTemporary)
// Double brackets trigger FileCheck variables, escape.
// CXX17-NEXT: 5: {{\[}}[B1.2] foo:[B1.4]]
Index: test/Analysis/cfg-rich-constructors.cpp
===================================================================
--- test/Analysis/cfg-rich-constructors.cpp
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -823,21 +823,49 @@
void useCByReference(const C &c);
void useD(D d);
void useDByReference(const D &d);
+void useCAndD(C c, D d);
-// FIXME: Find construction context for the argument.
// CHECK: void passArgument()
// CHECK: 1: useC
// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(class C))
-// CXX11-NEXT: 3: C() (CXXConstructExpr, [B1.4], class C)
+// CXX11-ELIDE-NEXT: 3: C() (CXXConstructExpr, [B1.4], [B1.5], class C)
+// CXX11-NOELIDE-NEXT: 3: C() (CXXConstructExpr, [B1.4], class C)
// CXX11-NEXT: 4: [B1.3]
-// CXX11-NEXT: 5: [B1.4] (CXXConstructExpr, class C)
+// CXX11-NEXT: 5: [B1.4] (CXXConstructExpr, [B1.6]+0, class C)
// CXX11-NEXT: 6: [B1.2]([B1.5])
-// CXX17-NEXT: 3: C() (CXXConstructExpr, class C)
+// CXX17-NEXT: 3: C() (CXXConstructExpr, [B1.4]+0, class C)
// CXX17-NEXT: 4: [B1.2]([B1.3])
void passArgument() {
useC(C());
}
+// CHECK: void passTwoArguments()
+// CHECK: [B1]
+// CHECK-NEXT: 1: useCAndD
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(class C, class argument_constructors::D))
+// CXX11-ELIDE-NEXT: 3: C() (CXXConstructExpr, [B1.4], [B1.5], class C)
+// CXX11-NOELIDE-NEXT: 3: C() (CXXConstructExpr, [B1.4], class C)
+// CXX11-NEXT: 4: [B1.3]
+// CXX11-NEXT: 5: [B1.4] (CXXConstructExpr, [B1.12]+0, class C)
+// CXX11-ELIDE-NEXT: 6: argument_constructors::D() (CXXConstructExpr, [B1.7], [B1.9], [B1.10], class argument_constructors::D)
+// CXX11-NOELIDE-NEXT: 6: argument_constructors::D() (CXXConstructExpr, [B1.7], [B1.9], class argument_constructors::D)
+// CXX11-NEXT: 7: [B1.6] (BindTemporary)
+// CXX11-NEXT: 8: [B1.7] (ImplicitCastExpr, NoOp, const class argument_constructors::D)
+// CXX11-NEXT: 9: [B1.8]
+// CXX11-NEXT: 10: [B1.9] (CXXConstructExpr, [B1.11], [B1.12]+1, class argument_constructors::D)
+// CXX11-NEXT: 11: [B1.10] (BindTemporary)
+// CXX11-NEXT: 12: [B1.2]([B1.5], [B1.11])
+// CXX11-NEXT: 13: ~argument_constructors::D() (Temporary object destructor)
+// CXX11-NEXT: 14: ~argument_constructors::D() (Temporary object destructor)
+// CXX17-NEXT: 3: C() (CXXConstructExpr, [B1.6]+0, class C)
+// CXX17-NEXT: 4: argument_constructors::D() (CXXConstructExpr, [B1.5], [B1.6]+1, class argument_co
+// CXX17-NEXT: 5: [B1.4] (BindTemporary)
+// CXX17-NEXT: 6: [B1.2]([B1.3], [B1.5])
+// CXX17-NEXT: 7: ~argument_constructors::D() (Temporary object destructor)
+void passTwoArguments() {
+ useCAndD(C(), D());
+}
+
// CHECK: void passArgumentByReference()
// CHECK: 1: useCByReference
// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(const class C &))
@@ -849,20 +877,20 @@
useCByReference(C());
}
-// FIXME: Find construction context for the argument.
// CHECK: void passArgumentWithDestructor()
// CHECK: 1: useD
// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(class argument_constructors::D))
-// CXX11-NEXT: 3: argument_constructors::D() (CXXConstructExpr, [B1.4], [B1.6], class argument_constructors::D)
+// CXX11-ELIDE-NEXT: 3: argument_constructors::D() (CXXConstructExpr, [B1.4], [B1.6], [B1.7], class argument_constructors::D)
+// CXX11-NOELIDE-NEXT: 3: argument_constructors::D() (CXXConstructExpr, [B1.4], [B1.6], class argument_constructors::D)
// CXX11-NEXT: 4: [B1.3] (BindTemporary)
// CXX11-NEXT: 5: [B1.4] (ImplicitCastExpr, NoOp, const class argument_constructors::D)
// CXX11-NEXT: 6: [B1.5]
-// CXX11-NEXT: 7: [B1.6] (CXXConstructExpr, class argument_constructors::D)
+// CXX11-NEXT: 7: [B1.6] (CXXConstructExpr, [B1.8], [B1.9]+0, class argument_constructors::D)
// CXX11-NEXT: 8: [B1.7] (BindTemporary)
// CXX11-NEXT: 9: [B1.2]([B1.8])
// CXX11-NEXT: 10: ~argument_constructors::D() (Temporary object destructor)
// CXX11-NEXT: 11: ~argument_constructors::D() (Temporary object destructor)
-// CXX17-NEXT: 3: argument_constructors::D() (CXXConstructExpr, class argument_constructors::D)
+// CXX17-NEXT: 3: argument_constructors::D() (CXXConstructExpr, [B1.4], [B1.5]+0, class argument_constructors::D)
// CXX17-NEXT: 4: [B1.3] (BindTemporary)
// CXX17-NEXT: 5: [B1.2]([B1.4])
// CXX17-NEXT: 6: ~argument_constructors::D() (Temporary object destructor)
@@ -883,13 +911,13 @@
useDByReference(D());
}
-// FIXME: Find construction context for the argument.
// CHECK: void passArgumentIntoAnotherConstructor()
-// CXX11: 1: argument_constructors::D() (CXXConstructExpr, [B1.2], [B1.4], class argument_constructors::D)
+// CXX11-ELIDE: 1: argument_constructors::D() (CXXConstructExpr, [B1.2], [B1.4], [B1.5], class argument_constructors::D)
+// CXX11-NOELIDE: 1: argument_constructors::D() (CXXConstructExpr, [B1.2], [B1.4], class argument_constructors::D)
// CXX11-NEXT: 2: [B1.1] (BindTemporary)
// CXX11-NEXT: 3: [B1.2] (ImplicitCastExpr, NoOp, const class argument_constructors::D)
// CXX11-NEXT: 4: [B1.3]
-// CXX11-NEXT: 5: [B1.4] (CXXConstructExpr, class argument_constructors::D)
+// CXX11-NEXT: 5: [B1.4] (CXXConstructExpr, [B1.6], [B1.7]+0, class argument_constructors::D)
// CXX11-NEXT: 6: [B1.5] (BindTemporary)
// CXX11-ELIDE-NEXT: 7: [B1.6] (CXXConstructExpr, [B1.9], [B1.10], class argument_constructors::E)
// CXX11-NOELIDE-NEXT: 7: [B1.6] (CXXConstructExpr, [B1.9], class argument_constructors::E)
@@ -899,7 +927,7 @@
// CXX11-NEXT: 11: argument_constructors::E e = argument_constructors::E(argument_constructors::D());
// CXX11-NEXT: 12: ~argument_constructors::D() (Temporary object destructor)
// CXX11-NEXT: 13: ~argument_constructors::D() (Temporary object destructor)
-// CXX17: 1: argument_constructors::D() (CXXConstructExpr, class argument_constructors::D)
+// CXX17: 1: argument_constructors::D() (CXXConstructExpr, [B1.2], [B1.3]+0, class argument_constructors::D)
// CXX17-NEXT: 2: [B1.1] (BindTemporary)
// CXX17-NEXT: 3: [B1.2] (CXXConstructExpr, [B1.5], class argument_constructors::E)
// CXX17-NEXT: 4: argument_constructors::E([B1.3]) (CXXFunctionalCastExpr, ConstructorConversion, class argument_constructors::E)
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -221,25 +221,42 @@
// and construct directly into the final object. This call
// also sets the CallOpts flags for us.
SVal V;
+ // If the elided copy/move constructor is not supported, there's still
+ // benefit in trying to model the non-elided constructor.
+ // Stash our state before trying to elide, as it'll get overwritten.
+ ProgramStateRef PreElideState = State;
+ EvalCallOptions PreElideCallOpts = CallOpts;
+
std::tie(State, V) = prepareForObjectConstruction(
CE, State, LCtx, TCC->getConstructionContextAfterElision(), CallOpts);
- // Remember that we've elided the constructor.
- State = addObjectUnderConstruction(State, CE, LCtx, V);
+ // FIXME: This definition of "copy elision has not failed" is unreliable.
+ // It doesn't indicate that the constructor will actually be inlined
+ // later; it is still up to evalCall() to decide.
+ if (!CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion) {
+ // Remember that we've elided the constructor.
+ State = addObjectUnderConstruction(State, CE, LCtx, V);
- // Remember that we've elided the destructor.
- if (BTE)
- State = elideDestructor(State, BTE, LCtx);
+ // Remember that we've elided the destructor.
+ if (BTE)
+ State = elideDestructor(State, BTE, LCtx);
- // Instead of materialization, shamelessly return
- // the final object destination.
- if (MTE)
- State = addObjectUnderConstruction(State, MTE, LCtx, V);
+ // Instead of materialization, shamelessly return
+ // the final object destination.
+ if (MTE)
+ State = addObjectUnderConstruction(State, MTE, LCtx, V);
- return std::make_pair(State, V);
+ return std::make_pair(State, V);
+ } else {
+ // Copy elision failed. Revert the changes and proceed as if we have
+ // a simple temporary.
+ State = PreElideState;
+ CallOpts = PreElideCallOpts;
+ }
+ // FALL-THROUGH
}
case ConstructionContext::SimpleTemporaryObjectKind: {
- const auto *TCC = cast<SimpleTemporaryObjectConstructionContext>(CC);
+ const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
SVal V = UnknownVal();
@@ -274,6 +291,10 @@
CallOpts.IsTemporaryCtorOrDtor = true;
return std::make_pair(State, V);
}
+ case ConstructionContext::ArgumentKind: {
+ // Function argument constructors. Not implemented yet.
+ break;
+ }
}
}
// If we couldn't find an existing region to construct into, assume we're
Index: lib/Analysis/ConstructionContext.cpp
===================================================================
--- lib/Analysis/ConstructionContext.cpp
+++ lib/Analysis/ConstructionContext.cpp
@@ -21,10 +21,11 @@
const ConstructionContextLayer *
ConstructionContextLayer::create(BumpVectorContext &C, TriggerTy Trigger,
+ unsigned Index,
const ConstructionContextLayer *Parent) {
ConstructionContextLayer *CC =
C.getAllocator().Allocate<ConstructionContextLayer>();
- return new (CC) ConstructionContextLayer(Trigger, Parent);
+ return new (CC) ConstructionContextLayer(Trigger, Index, Parent);
}
bool ConstructionContextLayer::isStrictlyMoreSpecificThan(
@@ -111,11 +112,14 @@
}
assert(ParentLayer->isLast());
- // This is a constructor into a function argument. Not implemented yet.
+ // This is a constructor into a function argument.
if (isa<CallExpr>(ParentLayer->getTriggerStmt()) ||
isa<CXXConstructExpr>(ParentLayer->getTriggerStmt()) ||
- isa<ObjCMessageExpr>(ParentLayer->getTriggerStmt()))
- return nullptr;
+ isa<ObjCMessageExpr>(ParentLayer->getTriggerStmt())) {
+ return create<ArgumentConstructionContext>(
+ C, cast<Expr>(ParentLayer->getTriggerStmt()),
+ ParentLayer->getIndex(), BTE);
+ }
// This is C++17 copy-elided construction into return statement.
if (auto *RS = dyn_cast<ReturnStmt>(ParentLayer->getTriggerStmt())) {
assert(!RS->getRetValue()->getType().getCanonicalType()
@@ -175,11 +179,15 @@
assert(TopLayer->isLast());
return create<SimpleReturnedValueConstructionContext>(C, RS);
}
- // This is a constructor into a function argument. Not implemented yet.
+ // This is a constructor into a function argument.
if (isa<CallExpr>(TopLayer->getTriggerStmt()) ||
isa<CXXConstructExpr>(TopLayer->getTriggerStmt()) ||
- isa<ObjCMessageExpr>(TopLayer->getTriggerStmt()))
- return nullptr;
+ isa<ObjCMessageExpr>(TopLayer->getTriggerStmt())) {
+ assert(TopLayer->isLast());
+ return create<ArgumentConstructionContext>(
+ C, cast<Expr>(TopLayer->getTriggerStmt()), TopLayer->getIndex(),
+ /*BTE=*/nullptr);
+ }
llvm_unreachable("Unexpected construction context with statement!");
} else if (const CXXCtorInitializer *I = TopLayer->getTriggerInit()) {
assert(TopLayer->isLast());
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -693,16 +693,13 @@
std::is_same<CallLikeExpr, CXXConstructExpr>::value ||
std::is_same<CallLikeExpr, ObjCMessageExpr>::value>>
void findConstructionContextsForArguments(CallLikeExpr *E) {
- // A stub for the code that'll eventually be used for finding construction
- // contexts for constructors of C++ object-type arguments passed into
- // call-like expression E.
- // FIXME: Once actually implemented, this construction context layer should
- // include the index of the argument as well.
- for (auto Arg : E->arguments())
+ for (unsigned i = 0, e = E->getNumArgs(); i != e; ++i) {
+ Expr *Arg = E->getArg(i);
if (Arg->getType()->getAsCXXRecordDecl() && !Arg->isGLValue())
findConstructionContexts(
- ConstructionContextLayer::create(cfg->getBumpVectorContext(), E),
+ ConstructionContextLayer::create(cfg->getBumpVectorContext(), E, i),
Arg);
+ }
}
// Unset the construction context after consuming it. This is done immediately
@@ -1289,9 +1286,9 @@
if (!Child)
return;
- auto withExtraLayer = [this, Layer](Stmt *S) {
+ auto withExtraLayer = [this, Layer](Stmt *S, unsigned Index = 0) {
return ConstructionContextLayer::create(cfg->getBumpVectorContext(), S,
- Layer);
+ Index, Layer);
};
switch(Child->getStmtClass()) {
@@ -5011,7 +5008,7 @@
OS << ", ";
const auto *SICC = cast<SimpleConstructorInitializerConstructionContext>(CC);
print_initializer(OS, Helper, SICC->getCXXCtorInitializer());
- break;
+ return;
}
case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind: {
OS << ", ";
@@ -5062,6 +5059,17 @@
Stmts.push_back(TOCC->getConstructorAfterElision());
break;
}
+ case ConstructionContext::ArgumentKind: {
+ const auto *ACC = cast<ArgumentConstructionContext>(CC);
+ if (const Stmt *BTE = ACC->getCXXBindTemporaryExpr()) {
+ OS << ", ";
+ Helper.handledStmt(const_cast<Stmt *>(BTE), OS);
+ }
+ OS << ", ";
+ Helper.handledStmt(const_cast<Expr *>(ACC->getCallLikeExpr()), OS);
+ OS << "+" << ACC->getIndex();
+ return;
+ }
}
for (auto I: Stmts)
if (I) {
Index: include/clang/Analysis/ConstructionContext.h
===================================================================
--- include/clang/Analysis/ConstructionContext.h
+++ include/clang/Analysis/ConstructionContext.h
@@ -41,6 +41,12 @@
/// new-expression triggers construction of the newly allocated object(s).
TriggerTy Trigger;
+ /// If a single trigger statement triggers multiple constructors, they are
+ /// usually being enumerated. This covers function argument constructors
+ /// triggered by a call-expression and items in an initializer list triggered
+ /// by an init-list-expression.
+ unsigned Index;
+
/// Sometimes a single trigger is not enough to describe the construction
/// site. In this case we'd have a chain of "partial" construction context
/// layers.
@@ -55,13 +61,13 @@
/// Not all of these are currently supported.
const ConstructionContextLayer *Parent = nullptr;
- ConstructionContextLayer(TriggerTy Trigger,
- const ConstructionContextLayer *Parent)
- : Trigger(Trigger), Parent(Parent) {}
+ ConstructionContextLayer(TriggerTy Trigger, unsigned Index,
+ const ConstructionContextLayer *Parent)
+ : Trigger(Trigger), Index(Index), Parent(Parent) {}
public:
static const ConstructionContextLayer *
- create(BumpVectorContext &C, TriggerTy Trigger,
+ create(BumpVectorContext &C, TriggerTy Trigger, unsigned Index = 0,
const ConstructionContextLayer *Parent = nullptr);
const ConstructionContextLayer *getParent() const { return Parent; }
@@ -75,11 +81,13 @@
return Trigger.dyn_cast<CXXCtorInitializer *>();
}
+ unsigned getIndex() const { return Index; }
+
/// Returns true if these layers are equal as individual layers, even if
/// their parents are different.
bool isSameLayer(const ConstructionContextLayer *Other) const {
assert(Other);
- return (Trigger == Other->Trigger);
+ return (Trigger == Other->Trigger && Index == Other->Index);
}
/// See if Other is a proper initial segment of this construction context
@@ -114,7 +122,8 @@
SimpleReturnedValueKind,
CXX17ElidedCopyReturnedValueKind,
RETURNED_VALUE_BEGIN = SimpleReturnedValueKind,
- RETURNED_VALUE_END = CXX17ElidedCopyReturnedValueKind
+ RETURNED_VALUE_END = CXX17ElidedCopyReturnedValueKind,
+ ArgumentKind
};
protected:
@@ -469,6 +478,32 @@
}
};
+class ArgumentConstructionContext : public ConstructionContext {
+ const Expr *CE; // The call of which the context is an argument.
+ unsigned Index; // Which argument we're constructing.
+ const CXXBindTemporaryExpr *BTE; // Whether the object needs to be destroyed.
+
+ friend class ConstructionContext; // Allows to create<>() itself.
+
+ explicit ArgumentConstructionContext(const Expr *CE, unsigned Index,
+ const CXXBindTemporaryExpr *BTE)
+ : ConstructionContext(ArgumentKind), CE(CE),
+ Index(Index), BTE(BTE) {
+ assert(isa<CallExpr>(CE) || isa<CXXConstructExpr>(CE) ||
+ isa<ObjCMessageExpr>(CE));
+ // BTE is optional.
+ }
+
+public:
+ const Expr *getCallLikeExpr() const { return CE; }
+ unsigned getIndex() const { return Index; }
+ const CXXBindTemporaryExpr *getCXXBindTemporaryExpr() const { return BTE; }
+
+ static bool classof(const ConstructionContext *CC) {
+ return CC->getKind() == ArgumentKind;
+ }
+};
+
} // end namespace clang
#endif // LLVM_CLANG_ANALYSIS_CONSTRUCTIONCONTEXT_H
Index: include/clang/Analysis/CFG.h
===================================================================
--- include/clang/Analysis/CFG.h
+++ include/clang/Analysis/CFG.h
@@ -196,7 +196,8 @@
// These are possible in C++17 due to mandatory copy elision.
isa<ReturnedValueConstructionContext>(C) ||
isa<VariableConstructionContext>(C) ||
- isa<ConstructorInitializerConstructionContext>(C)));
+ isa<ConstructorInitializerConstructionContext>(C) ||
+ isa<ArgumentConstructionContext>(C)));
Data2.setPointer(const_cast<ConstructionContext *>(C));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
