NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
Temporaries are destroyed at the end of their `CXXBindTemporaryExpr`, which can
be picked up from their `CFGTemporaryDtor`. Note that lifetime-extended
temporaries are different: they are destroyed via `CFGAutomaticObjectDtor` for
their reference, and they don't have a `CFGTemoraryDtor` of their own. Unless,
well, they are copied with an elidable copy-constructor, in which case the
temporary destructor is there, but it still fires immediately, and
lifetime-extended copy is still dealt with via an automatic destructor.
I think this is one of the places where diagnostics may become confusing due to
presence of elidable constructors and their respective destructors. In the
second test case nobody would expect anything to be destroyed on the line on
which we show the "Calling '~C'" note. So we may need to either think of
actually eliding elidable constructors together with their destructors, or at
least explaining to the user that the constructor is elidable as part of the
path note.
Repository:
rC Clang
https://reviews.llvm.org/D43144
Files:
lib/StaticAnalyzer/Core/PathDiagnostic.cpp
test/Analysis/inlining/temp-dtors-path-notes.cpp
Index: test/Analysis/inlining/temp-dtors-path-notes.cpp
===================================================================
--- /dev/null
+++ test/Analysis/inlining/temp-dtors-path-notes.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config
cfg-temporary-dtors=true -analyzer-output=text -verify %s
+
+namespace test_simple_temporary {
+class C {
+ int x;
+
+public:
+ C(int x): x(x) {} // expected-note{{The value 0 is assigned to field 'x'}}
+ ~C() { x = 1 / x; } // expected-warning{{Division by zero}}
+ // expected-note@-1{{Division by zero}}
+};
+
+void test() {
+ C(0); // expected-note {{Passing the value 0 via 1st parameter 'x'}}
+ // expected-note@-1{{Calling constructor for 'C'}}
+ // expected-note@-2{{Returning from constructor for 'C'}}
+ // expected-note@-3{{Calling '~C'}}
+}
+} // end namespace test_simple_temporary
+
+namespace test_lifetime_extended_temporary {
+class C {
+ int x;
+
+public:
+ C(int x): x(x) {} // expected-note{{The value 0 is assigned to field 'x'}}
+ void nop() const {}
+ ~C() { x = 1 / x; } // expected-warning{{Division by zero}}
+ // expected-note@-1{{Division by zero}}
+};
+
+void test(int coin) {
+ // We'd divide by zero in the temporary destructor that goes after the
+ // elidable copy-constructor from C(0) to the lifetime-extended temporary.
+ // So in fact this example has nothing to do with lifetime extension.
+ // Actually, it would probably be better to elide the constructor, and
+ // put the note for the destructor call at the closing brace after nop.
+ const C &c = coin ? C(1) : C(0); // expected-note{{Assuming 'coin' is 0}}
+ // expected-note@-1{{'?' condition is
false}}
+ // expected-note@-2{{Passing the value 0
via 1st parameter 'x'}}
+ // expected-note@-3{{Calling constructor
for 'C'}}
+ // expected-note@-4{{Returning from
constructor for 'C'}}
+ // expected-note@-5{{Calling '~C'}}
+ c.nop();
+}
+} // end namespace test_lifetime_extended_temporary
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -579,8 +579,14 @@
const CFGNewAllocator &Alloc = Source.castAs<CFGNewAllocator>();
return PathDiagnosticLocation(Alloc.getAllocatorExpr(), SM, CallerCtx);
}
- case CFGElement::TemporaryDtor:
- llvm_unreachable("not yet implemented!");
+ case CFGElement::TemporaryDtor: {
+ // Temporary destructors are for temporaries. They die immediately at
around
+ // the location of CXXBindTemporaryExpr. If they are lifetime-extended,
+ // they'd be dealt with via an AutomaticObjectDtor instead.
+ const CFGTemporaryDtor &Dtor = Source.castAs<CFGTemporaryDtor>();
+ return PathDiagnosticLocation::createEnd(Dtor.getBindTemporaryExpr(), SM,
+ CallerCtx);
+ }
case CFGElement::LifetimeEnds:
case CFGElement::LoopExit:
llvm_unreachable("CFGElement kind should not be on callsite!");
Index: test/Analysis/inlining/temp-dtors-path-notes.cpp
===================================================================
--- /dev/null
+++ test/Analysis/inlining/temp-dtors-path-notes.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config cfg-temporary-dtors=true -analyzer-output=text -verify %s
+
+namespace test_simple_temporary {
+class C {
+ int x;
+
+public:
+ C(int x): x(x) {} // expected-note{{The value 0 is assigned to field 'x'}}
+ ~C() { x = 1 / x; } // expected-warning{{Division by zero}}
+ // expected-note@-1{{Division by zero}}
+};
+
+void test() {
+ C(0); // expected-note {{Passing the value 0 via 1st parameter 'x'}}
+ // expected-note@-1{{Calling constructor for 'C'}}
+ // expected-note@-2{{Returning from constructor for 'C'}}
+ // expected-note@-3{{Calling '~C'}}
+}
+} // end namespace test_simple_temporary
+
+namespace test_lifetime_extended_temporary {
+class C {
+ int x;
+
+public:
+ C(int x): x(x) {} // expected-note{{The value 0 is assigned to field 'x'}}
+ void nop() const {}
+ ~C() { x = 1 / x; } // expected-warning{{Division by zero}}
+ // expected-note@-1{{Division by zero}}
+};
+
+void test(int coin) {
+ // We'd divide by zero in the temporary destructor that goes after the
+ // elidable copy-constructor from C(0) to the lifetime-extended temporary.
+ // So in fact this example has nothing to do with lifetime extension.
+ // Actually, it would probably be better to elide the constructor, and
+ // put the note for the destructor call at the closing brace after nop.
+ const C &c = coin ? C(1) : C(0); // expected-note{{Assuming 'coin' is 0}}
+ // expected-note@-1{{'?' condition is false}}
+ // expected-note@-2{{Passing the value 0 via 1st parameter 'x'}}
+ // expected-note@-3{{Calling constructor for 'C'}}
+ // expected-note@-4{{Returning from constructor for 'C'}}
+ // expected-note@-5{{Calling '~C'}}
+ c.nop();
+}
+} // end namespace test_lifetime_extended_temporary
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -579,8 +579,14 @@
const CFGNewAllocator &Alloc = Source.castAs<CFGNewAllocator>();
return PathDiagnosticLocation(Alloc.getAllocatorExpr(), SM, CallerCtx);
}
- case CFGElement::TemporaryDtor:
- llvm_unreachable("not yet implemented!");
+ case CFGElement::TemporaryDtor: {
+ // Temporary destructors are for temporaries. They die immediately at around
+ // the location of CXXBindTemporaryExpr. If they are lifetime-extended,
+ // they'd be dealt with via an AutomaticObjectDtor instead.
+ const CFGTemporaryDtor &Dtor = Source.castAs<CFGTemporaryDtor>();
+ return PathDiagnosticLocation::createEnd(Dtor.getBindTemporaryExpr(), SM,
+ CallerCtx);
+ }
case CFGElement::LifetimeEnds:
case CFGElement::LoopExit:
llvm_unreachable("CFGElement kind should not be on callsite!");
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
