NoQ updated this revision to Diff 140464.
NoQ added a comment.
Use auto.
https://reviews.llvm.org/D44955
Files:
include/clang/Analysis/ConstructionContext.h
lib/Analysis/CFG.cpp
lib/Analysis/ConstructionContext.cpp
test/Analysis/missing-bind-temporary.cpp
Index: test/Analysis/missing-bind-temporary.cpp
===================================================================
--- /dev/null
+++ test/Analysis/missing-bind-temporary.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++14 -verify %s
+
+void clang_analyzer_eval(bool);
+
+int global;
+
+namespace variant_0 {
+// This variant of the code works correctly. Function foo() is not a template
+// function. Note that there are two destructors within foo().
+
+class A {
+public:
+ ~A() { ++global; }
+};
+
+class B {
+ A a;
+};
+
+// CHECK: void foo(int)
+// CHECK: [B1]
+// CHECK-NEXT: 1: (CXXConstructExpr, [B1.2], class variant_0::B)
+// CHECK-NEXT: 2: variant_0::B i;
+// CHECK-NEXT: 3: operator=
+// CHECK-NEXT: 4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, class variant_0::B &(*)(class variant_0::B &&) noexcept)
+// CHECK-NEXT: 5: i
+// CHECK-NEXT: 6: {} (CXXConstructExpr, [B1.7], [B1.8], class variant_0::B)
+// CHECK-NEXT: 7: [B1.6] (BindTemporary)
+// CHECK-NEXT: 8: [B1.7]
+// CHECK-NEXT: 9: [B1.5] = [B1.8] (OperatorCall)
+// CHECK-NEXT: 10: ~variant_0::B() (Temporary object destructor)
+// CHECK-NEXT: 11: [B1.2].~B() (Implicit destructor)
+void foo(int) {
+ B i;
+ i = {};
+}
+
+void bar() {
+ global = 0;
+ foo(1);
+ clang_analyzer_eval(global == 2); // expected-warning{{TRUE}}
+}
+
+} // end namespace variant_0
+
+namespace variant_1 {
+// Suddenly, if we turn foo() into a template, we are missing a
+// CXXBindTemporaryExpr in the AST, and therefore we're missing a
+// temporary destructor in the CFG.
+
+class A {
+public:
+ ~A() { ++global; }
+};
+
+class B {
+ A a;
+};
+
+// FIXME: Find the construction context for {} and enforce the temporary
+// destructor.
+// CHECK: template<> void foo<int>(int)
+// CHECK: [B1]
+// CHECK-NEXT: 1: (CXXConstructExpr, [B1.2], class variant_1::B)
+// CHECK-NEXT: 2: variant_1::B i;
+// CHECK-NEXT: 3: operator=
+// CHECK-NEXT: 4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, class variant_1::B &(*)(class variant_1::B &&) noexcept)
+// CHECK-NEXT: 5: i
+// CHECK-NEXT: 6: {} (CXXConstructExpr, class variant_1::B)
+// CHECK-NEXT: 7: [B1.6]
+// CHECK-NEXT: 8: [B1.5] = [B1.7] (OperatorCall)
+// CHECK-NEXT: 9: [B1.2].~B() (Implicit destructor)
+template <typename T> void foo(T) {
+ B i;
+ i = {};
+}
+
+void bar() {
+ global = 0;
+ foo(1);
+ // FIXME: Should be TRUE, i.e. we should call (and inline) two destructors.
+ clang_analyzer_eval(global == 2); // expected-warning{{UNKNOWN}}
+}
+
+} // end namespace variant_1
+
+namespace variant_2 {
+// Making field 'a' in class 'B' public turns the class into an aggregate.
+// In this case there is no constructor at {} - only an aggregate
+// initialization. Aggregate initialization is unsupported for now.
+
+class A {
+public:
+ ~A() { ++global; }
+};
+
+class B {
+public:
+ A a;
+};
+
+// CHECK: template<> void foo<int>(int)
+// CHECK: [B1]
+// CHECK-NEXT: 1: (CXXConstructExpr, [B1.2], class variant_2::B)
+// CHECK-NEXT: 2: variant_2::B i;
+// CHECK-NEXT: 3: operator=
+// CHECK-NEXT: 4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, class variant_2::B &(*)(class variant_2::B &&) noexcept)
+// CHECK-NEXT: 5: i
+// CHECK-NEXT: 6: {}
+// CHECK-NEXT: 7: {}
+// CHECK-NEXT: 8: [B1.7] (BindTemporary)
+// CHECK-NEXT: 9: [B1.8]
+// CHECK-NEXT: 10: [B1.5] = [B1.9] (OperatorCall)
+// CHECK-NEXT: 11: ~variant_2::B() (Temporary object destructor)
+// CHECK-NEXT: 12: [B1.2].~B() (Implicit destructor)
+template <typename T> void foo(T) {
+ B i;
+ i = {};
+}
+
+void bar() {
+ global = 0;
+ foo(1);
+ // FIXME: Should be TRUE, i.e. we should call (and inline) two destructors.
+ clang_analyzer_eval(global == 2); // expected-warning{{UNKNOWN}}
+}
+
+} // end namespace variant_2
Index: lib/Analysis/ConstructionContext.cpp
===================================================================
--- lib/Analysis/ConstructionContext.cpp
+++ lib/Analysis/ConstructionContext.cpp
@@ -101,9 +101,12 @@
if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(S)) {
// If the object requires destruction and is not lifetime-extended,
// then it must have a BTE within its MTE.
- assert(MTE->getType().getCanonicalType()
+ // FIXME: This should be an assertion.
+ if (!(MTE->getType().getCanonicalType()
->getAsCXXRecordDecl()->hasTrivialDestructor() ||
- MTE->getStorageDuration() != SD_FullExpression);
+ MTE->getStorageDuration() != SD_FullExpression))
+ return nullptr;
+
assert(TopLayer->isLast());
return create<TemporaryObjectConstructionContext>(C, nullptr, MTE);
}
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -736,12 +736,12 @@
if (BuildOpts.AddRichCXXConstructors) {
if (const ConstructionContextLayer *Layer =
ConstructionContextMap.lookup(CE)) {
- const ConstructionContext *CC =
- ConstructionContext::createFromLayers(cfg->getBumpVectorContext(),
- Layer);
- B->appendConstructor(CE, CC, cfg->getBumpVectorContext());
cleanupConstructionContext(CE);
- return;
+ if (const auto *CC = ConstructionContext::createFromLayers(
+ cfg->getBumpVectorContext(), Layer)) {
+ B->appendConstructor(CE, CC, cfg->getBumpVectorContext());
+ return;
+ }
}
}
@@ -757,12 +757,12 @@
if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context)) {
if (const ConstructionContextLayer *Layer =
ConstructionContextMap.lookup(CE)) {
- const ConstructionContext *CC =
- ConstructionContext::createFromLayers(cfg->getBumpVectorContext(),
- Layer);
- B->appendCXXRecordTypedCall(CE, CC, cfg->getBumpVectorContext());
cleanupConstructionContext(CE);
- return;
+ if (const auto *CC = ConstructionContext::createFromLayers(
+ cfg->getBumpVectorContext(), Layer)) {
+ B->appendCXXRecordTypedCall(CE, CC, cfg->getBumpVectorContext());
+ return;
+ }
}
}
}
Index: include/clang/Analysis/ConstructionContext.h
===================================================================
--- include/clang/Analysis/ConstructionContext.h
+++ include/clang/Analysis/ConstructionContext.h
@@ -131,7 +131,8 @@
public:
/// Consume the construction context layer, together with its parent layers,
- /// and wrap it up into a complete construction context.
+ /// and wrap it up into a complete construction context. May return null
+ /// if layers do not form any supported construction context.
static const ConstructionContext *
createFromLayers(BumpVectorContext &C,
const ConstructionContextLayer *TopLayer);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
