[PATCH] D43804: [analyzer] Enable cfg-temporary-dtors by default?
This revision was automatically updated to reflect the committed changes.
Closed by commit rL326461: [analyzer] Enable cfg-temporary-dtors by default.
(authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43804?vs=136428=136578#toc
Repository:
rL LLVM
https://reviews.llvm.org/D43804
Files:
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/analyzer-config.cpp
cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
cfe/trunk/test/Analysis/lifetime-cfg-output.cpp
cfe/trunk/test/Analysis/lifetime-extension.cpp
cfe/trunk/test/Analysis/temporaries.cpp
Index: cfe/trunk/test/Analysis/lifetime-cfg-output.cpp
===
--- cfe/trunk/test/Analysis/lifetime-cfg-output.cpp
+++ cfe/trunk/test/Analysis/lifetime-cfg-output.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-lifetime=true,cfg-rich-constructors=false -analyzer-config cfg-implicit-dtors=false %s > %t 2>&1
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-lifetime=true,cfg-temporary-dtors=false,cfg-rich-constructors=false -analyzer-config cfg-implicit-dtors=false %s > %t 2>&1
// RUN: FileCheck --input-file=%t %s
extern bool UV;
Index: cfe/trunk/test/Analysis/analyzer-config.cpp
===
--- cfe/trunk/test/Analysis/analyzer-config.cpp
+++ cfe/trunk/test/Analysis/analyzer-config.cpp
@@ -13,7 +13,7 @@
class Foo {
public:
~Foo() {}
- void baz();
+ void baz() { Foo(); }
void bar() { const Foo = Foo(); }
void foo() { bar(); }
};
@@ -23,13 +23,14 @@
// CHECK-NEXT: c++-inlining = destructors
// CHECK-NEXT: c++-shared_ptr-inlining = false
// CHECK-NEXT: c++-stdlib-inlining = true
+// CHECK-NEXT: c++-temp-dtor-inlining = false
// CHECK-NEXT: c++-template-inlining = true
// CHECK-NEXT: cfg-conditional-static-initializers = true
// CHECK-NEXT: cfg-implicit-dtors = true
// CHECK-NEXT: cfg-lifetime = false
// CHECK-NEXT: cfg-loopexit = false
// CHECK-NEXT: cfg-rich-constructors = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
// CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
// CHECK-NEXT: exploration_strategy = unexplored_first_queue
// CHECK-NEXT: faux-bodies = true
@@ -48,4 +49,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 28
+// CHECK-NEXT: num-entries = 29
Index: cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
===
--- cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
+++ cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config cfg-temporary-dtors=true -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -analyzer-output=text -verify %s
namespace test_simple_temporary {
class C {
Index: cfe/trunk/test/Analysis/lifetime-extension.cpp
===
--- cfe/trunk/test/Analysis/lifetime-extension.cpp
+++ cfe/trunk/test/Analysis/lifetime-extension.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -verify %s
-// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true -DTEMPORARIES -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -DTEMPORARIES -verify %s
void clang_analyzer_eval(bool);
Index: cfe/trunk/test/Analysis/analyzer-config.c
===
--- cfe/trunk/test/Analysis/analyzer-config.c
+++ cfe/trunk/test/Analysis/analyzer-config.c
@@ -16,7 +16,7 @@
// CHECK-NEXT: cfg-lifetime = false
// CHECK-NEXT: cfg-loopexit = false
// CHECK-NEXT: cfg-rich-constructors = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
// CHECK-NEXT: exploration_strategy = unexplored_first_queue
// CHECK-NEXT: faux-bodies = true
// CHECK-NEXT: graph-trim-interval = 1000
Index: cfe/trunk/test/Analysis/temporaries.cpp
===
--- cfe/trunk/test/Analysis/temporaries.cpp
+++
[PATCH] D43804: [analyzer] Enable cfg-temporary-dtors by default?
dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. Yay! This looks good to me. https://reviews.llvm.org/D43804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43804: [analyzer] Enable cfg-temporary-dtors by default?
NoQ updated this revision to Diff 136428.
NoQ added a comment.
Don't inline temporary destructors for now (i.e. keep
`c++-temp-dtor-inlining=false`, previously it was by default irrelevant and
arbitrarily set to true). That's because https://reviews.llvm.org/D43791 wasn't
enough to handle all the problems with smart pointers; some custom reference
counting pointers produce large inlining stacks that we just refuse to
traverse. I'd try to address it sooner rather than later and then enable this
flag as well.
With `c++-temp-dtor-inlining=false` this change seems even more quiet (around
+27/-24, on roughly the same codebase that produced +20/-35 in
https://reviews.llvm.org/D42219), and much like the last time it's mostly about
rearranging large chunks of false positives due to complete lack of support for
temporaries into smaller groups of false positives due to lack of support for
something else. This is far from a breakthrough yet - a lot more work needs to
be done, but neither it is falling apart immediately, and, most importantly, it
gives hope to address any single problem with a targeted fix, because
fundamental issues we used to have with temporaries would be mostly gone. So i
wish to encourage switching to the new mode earlier rather than later (though
it's always possible to flip the flag back locally in case of any severe
problems i didn't foresee).
https://reviews.llvm.org/D43804
Files:
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
test/Analysis/analyzer-config.c
test/Analysis/analyzer-config.cpp
test/Analysis/inlining/temp-dtors-path-notes.cpp
test/Analysis/lifetime-cfg-output.cpp
test/Analysis/lifetime-extension.cpp
test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -1,6 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true %s -std=c++11
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++11
extern bool clang_analyzer_eval(bool);
extern bool clang_analyzer_warnIfReached();
Index: test/Analysis/lifetime-extension.cpp
===
--- test/Analysis/lifetime-extension.cpp
+++ test/Analysis/lifetime-extension.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -verify %s
-// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true -DTEMPORARIES -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -DTEMPORARIES -verify %s
void clang_analyzer_eval(bool);
Index: test/Analysis/lifetime-cfg-output.cpp
===
--- test/Analysis/lifetime-cfg-output.cpp
+++ test/Analysis/lifetime-cfg-output.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-lifetime=true,cfg-rich-constructors=false -analyzer-config cfg-implicit-dtors=false %s > %t 2>&1
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-lifetime=true,cfg-temporary-dtors=false,cfg-rich-constructors=false -analyzer-config cfg-implicit-dtors=false %s > %t 2>&1
// RUN: FileCheck --input-file=%t %s
extern bool UV;
Index: test/Analysis/inlining/temp-dtors-path-notes.cpp
===
--- test/Analysis/inlining/temp-dtors-path-notes.cpp
+++ test/Analysis/inlining/temp-dtors-path-notes.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config cfg-temporary-dtors=true -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -analyzer-output=text -verify %s
namespace test_simple_temporary {
class C {
Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -13,7 +13,7 @@
class Foo {
[PATCH] D43804: [analyzer] Enable cfg-temporary-dtors by default?
NoQ added a comment. That's right, most analyzer-oriented tests were added explicitly by me to `temporaries.cpp` and `lifetime-extension.cpp` in previous patches, so they didn't need to be changed in this patch. There didn't seem to be many FIXME tests specific to my work before i started doing it - it seems that i covered all of them by adding a `cfg-temporary-dtors=true` run-line in previous patches. Repository: rC Clang https://reviews.llvm.org/D43804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43804: [analyzer] Enable cfg-temporary-dtors by default?
george.karpenkov added a comment. Currently there's no test demonstrating a different behavior from `cfg-temporary-dtors` being set to true?.. Repository: rC Clang https://reviews.llvm.org/D43804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43804: [analyzer] Enable cfg-temporary-dtors by default?
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
Final results are still being baked on my end, but by putting this on review
i'm trying to say that i believe i managed to fix the biggest problems with
this mode with the recent patches, to the point where the new mode is not worse
than the old mode. There are still known issues, such as the loss of coverage
due to default arguments of object or reference type, but these seem fairly
minor and will hopefully be addressed incrementally later on.
The change is not very loud - it's 2-3 times more impactful than the operator
new support we added earlier, which is noticeable but not definitely extreme.
The new mode fixes some but definitely not all false positives i wanted it to
fix - adding support for more constructors is necessary, and i hope to continue
improving it in the nearest future. It also provides a fairly fair amount of
neat true positives and an expected skew due to the changes in inlining.
Repository:
rC Clang
https://reviews.llvm.org/D43804
Files:
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
test/Analysis/analyzer-config.c
test/Analysis/analyzer-config.cpp
test/Analysis/lifetime-cfg-output.cpp
test/Analysis/lifetime-extension.cpp
Index: test/Analysis/lifetime-extension.cpp
===
--- test/Analysis/lifetime-extension.cpp
+++ test/Analysis/lifetime-extension.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11
-analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11
-analyzer-checker=core,debug.ExprInspection -analyzer-config
cfg-temporary-dtors=false -verify %s
// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11
-analyzer-checker=core,debug.ExprInspection -analyzer-config
cfg-temporary-dtors=true -DTEMPORARIES -verify %s
void clang_analyzer_eval(bool);
Index: test/Analysis/lifetime-cfg-output.cpp
===
--- test/Analysis/lifetime-cfg-output.cpp
+++ test/Analysis/lifetime-cfg-output.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze
-analyzer-checker=debug.DumpCFG -analyzer-config
cfg-lifetime=true,cfg-rich-constructors=false -analyzer-config
cfg-implicit-dtors=false %s > %t 2>&1
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze
-analyzer-checker=debug.DumpCFG -analyzer-config
cfg-lifetime=true,cfg-temporary-dtors=false,cfg-rich-constructors=false
-analyzer-config cfg-implicit-dtors=false %s > %t 2>&1
// RUN: FileCheck --input-file=%t %s
extern bool UV;
Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -13,7 +13,7 @@
class Foo {
public:
~Foo() {}
- void baz();
+ void baz() { Foo(); }
void bar() { const Foo = Foo(); }
void foo() { bar(); }
};
@@ -23,13 +23,14 @@
// CHECK-NEXT: c++-inlining = destructors
// CHECK-NEXT: c++-shared_ptr-inlining = false
// CHECK-NEXT: c++-stdlib-inlining = true
+// CHECK-NEXT: c++-temp-dtor-inlining = true
// CHECK-NEXT: c++-template-inlining = true
// CHECK-NEXT: cfg-conditional-static-initializers = true
// CHECK-NEXT: cfg-implicit-dtors = true
// CHECK-NEXT: cfg-lifetime = false
// CHECK-NEXT: cfg-loopexit = false
// CHECK-NEXT: cfg-rich-constructors = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
// CHECK-NEXT: exploration_strategy = dfs
// CHECK-NEXT: faux-bodies = true
// CHECK-NEXT: graph-trim-interval = 1000
@@ -47,4 +48,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 27
+// CHECK-NEXT: num-entries = 28
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -16,7 +16,7 @@
// CHECK-NEXT: cfg-lifetime = false
// CHECK-NEXT: cfg-loopexit = false
// CHECK-NEXT: cfg-rich-constructors = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
// CHECK-NEXT: exploration_strategy = dfs
// CHECK-NEXT: faux-bodies = true
// CHECK-NEXT: graph-trim-interval = 1000
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -191,7 +191,7 @@
bool AnalyzerOptions::includeTemporaryDtorsInCFG() {
return getBooleanOption(IncludeTemporaryDtorsInCFG,
"cfg-temporary-dtors",
- /* Default = */ false);
+ /* Default = */ true);
}
bool
