[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348031: [analyzer] Evaluate all non-checker config options 
before analysis (authored by Szelethus, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53692?vs=172856=176183#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53692/new/

https://reviews.llvm.org/D53692

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp

Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -204,7 +204,7 @@
 PP(CI.getPreprocessor()), OutDir(outdir), Opts(std::move(opts)),
 Plugins(plugins), Injector(injector), CTU(CI) {
 DigestAnalyzerOptions();
-if (Opts->PrintStats || Opts->shouldSerializeStats()) {
+if (Opts->PrintStats || Opts->ShouldSerializeStats) {
   AnalyzerTimers = llvm::make_unique(
   "analyzer", "Analyzer timers");
   TUTotalTimer = llvm::make_unique(
@@ -739,7 +739,7 @@
 
   // Execute the worklist algorithm.
   Eng.ExecuteWorkList(Mgr->getAnalysisDeclContextManager().getStackFrame(D),
-  Mgr->options.getMaxNodesPerTopLevelFunction());
+  Mgr->options.MaxNodesPerTopLevelFunction);
 
   if (!Mgr->options.DumpExplodedGraphTo.empty())
 Eng.DumpGraph(Mgr->options.TrimGraph, Mgr->options.DumpExplodedGraphTo);
Index: lib/StaticAnalyzer/Frontend/ModelInjector.cpp
===
--- lib/StaticAnalyzer/Frontend/ModelInjector.cpp
+++ lib/StaticAnalyzer/Frontend/ModelInjector.cpp
@@ -48,7 +48,7 @@
   FileID mainFileID = SM.getMainFileID();
 
   AnalyzerOptionsRef analyzerOpts = CI.getAnalyzerOpts();
-  llvm::StringRef modelPath = analyzerOpts->getModelPath();
+  llvm::StringRef modelPath = analyzerOpts->ModelPath;
 
   llvm::SmallString<128> fileName;
 
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -385,7 +385,7 @@
   // instead of generating an Unknown value and propagate the taint info to it.
   const unsigned MaxComp = StateMgr.getOwningEngine()
->getAnalysisManager()
-   .options.getMaxSymbolComplexity();
+   .options.MaxSymbolComplexity;
 
   if (symLHS && symRHS &&
   (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -676,8 +676,8 @@
 bool EnableNullFPSuppression, BugReport ,
 const SVal V) {
 AnalyzerOptions  = N->getState()->getAnalysisManager().options;
-if (EnableNullFPSuppression && Options.shouldSuppressNullReturnPaths()
-  && V.getAs())
+if (EnableNullFPSuppression &&
+Options.ShouldSuppressNullReturnPaths && V.getAs())
   BR.addVisitor(llvm::make_unique(
   R->getAs(), V));
   }
@@ -808,7 +808,8 @@
 AnalyzerOptions  = State->getAnalysisManager().options;
 
 bool EnableNullFPSuppression = false;
-if (InEnableNullFPSuppression && Options.shouldSuppressNullReturnPaths())
+if (InEnableNullFPSuppression &&
+Options.ShouldSuppressNullReturnPaths)
   if (Optional RetLoc = RetVal.getAs())
 EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
 
@@ -877,7 +878,7 @@
 // future nodes. We want to emit a path note as well, in case
 // the report is resurrected as valid later on.
 if (EnableNullFPSuppression &&
-Options.shouldAvoidSuppressingNullArgumentPaths())
+

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

In D53692#1313956 , @NoQ wrote:

> In D53692#1293781 , @NoQ wrote:
>
> > In D53692#1293778 , @Szelethus 
> > wrote:
> >
> > > Did you know that clang unit tests do not run with check-clang-analysis?
> >
> >
> > Well, *some* unit tests definitely do run under check-clang-analysis.
>
>
> I think they're actually //compiled// but indeed not //run//. Ugh. I guess it 
> should be fixed.


Uhh, yea, forgot to send my comment with the same observation. I tried to fix 
it myself, but a day and a half of CMake made me appreciate even preprocessor 
macros more then ever.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53692/new/

https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.
Herald added subscribers: gamesh411, baloghadamsoftware.

In D53692#1293781 , @NoQ wrote:

> In D53692#1293778 , @Szelethus wrote:
>
> > Did you know that clang unit tests do not run with check-clang-analysis?
>
>
> Well, *some* unit tests definitely do run under check-clang-analysis.


I think they're actually //compiled// but indeed not //run//. Ugh. I guess it 
should be fixed.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53692/new/

https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D53692#1293778, @Szelethus wrote:

> Did you know that clang unit tests do not run with check-clang-analysis?


Well, *some* unit tests definitely do run under check-clang-analysis.


Repository:
  rC Clang

https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus marked an inline comment as done.
Szelethus added a comment.

Thanks! This patch was the last for a while directly related to non-checker 
config options, I'm currently stuck on them as I unearthed countless bugs that 
I have to fix beforehand. (Did you know that clang unit tests do not run with 
check-clang-analysis?)




Comment at: test/Analysis/analyzer-config.cpp:1
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null 
-analyzer-checker=core,osx.cocoa,debug.ConfigDumper -analyzer-max-loop 34 > %t 
2>&1
 // RUN: FileCheck --input-file=%t %s --match-full-lines

NoQ wrote:
> NoQ wrote:
> > Aaand this test should probably be removed entirely.
> I think this one's not actually done^^
Uh, right, I misunderstood what you meant :D


Repository:
  rC Clang

https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Thanks, this is really wonderful.




Comment at: test/Analysis/analyzer-config.cpp:1
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null 
-analyzer-checker=core,osx.cocoa,debug.ConfigDumper -analyzer-max-loop 34 > %t 
2>&1
 // RUN: FileCheck --input-file=%t %s --match-full-lines

NoQ wrote:
> Aaand this test should probably be removed entirely.
I think this one's not actually done^^


Repository:
  rC Clang

https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 172856.
Szelethus set the repository for this revision to rC Clang.
Szelethus added a comment.

- Moved initializer functions to `CompilerInvocation.cpp`, like every other 
Options-like class.
- The fields for options are no longer `Optional`
- Fixed the test files


Repository:
  rC Clang

https://reviews.llvm.org/D53692

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp

Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -1,24 +1,10 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null -analyzer-checker=core,osx.cocoa,debug.ConfigDumper -analyzer-max-loop 34 > %t 2>&1
 // RUN: FileCheck --input-file=%t %s --match-full-lines
 
-void bar() {}
-void foo() {
-  // Call bar 33 times so max-times-inline-large is met and
-  // min-blocks-for-inline-large is checked
-  for (int i = 0; i < 34; ++i) {
-bar();
-  }
-}
-
-class Foo {
-public:
-  ~Foo() {}
-  void baz() { Foo(); }
-	void bar() { const Foo  = Foo(); }
-	void foo() { bar(); }
-};
-
 // CHECK: [config]
+// CHECK-NEXT: aggressive-binary-operation-simplification = false
+// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
+// CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
@@ -32,8 +18,12 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: crosscheck-with-z3 = false
+// CHECK-NEXT: ctu-dir = ""
+// CHECK-NEXT: ctu-index-name = externalFnMap.txt
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
+// CHECK-NEXT:expand-macros = false
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
@@ -43,12 +33,22 @@
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: max-inlinable-size = 100
 // CHECK-NEXT: max-nodes = 225000
+// CHECK-NEXT: max-symbol-complexity = 35
 // CHECK-NEXT: max-times-inline-large = 32
 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
+// CHECK-NEXT: model-path = ""
+// CHECK-NEXT: notes-as-events = false
+// CHECK-NEXT: objc-inlining = true
+// CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2
+// CHECK-NEXT: report-in-main-source-file = false
 // CHECK-NEXT: serialize-stats = false
+// CHECK-NEXT: stable-report-filename = false
+// CHECK-NEXT: suppress-c++-stdlib = true
+// CHECK-NEXT: suppress-inlined-defensive-checks = true
+// CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 31
+// CHECK-NEXT: num-entries = 48
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -1,39 +1,54 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null -analyzer-checker=core,osx.cocoa,debug.ConfigDumper -analyzer-max-loop 34 > %t 2>&1
 // RUN: FileCheck --input-file=%t %s --match-full-lines
 
-void bar() {}
-void foo() {
-  // Call bar 33 times so max-times-inline-large is met and
-  // min-blocks-for-inline-large is checked
-  for (int i = 0; i < 34; ++i) {
-bar();
-  }
-}
-
 // CHECK: [config]
+// CHECK-NEXT: aggressive-binary-operation-simplification = false
+// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
+// CHECK-NEXT: c++-allocator-inlining = true
+// CHECK-NEXT: c++-container-inlining = false
+// 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: 

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus marked 2 inline comments as done.
Szelethus added inline comments.



Comment at: test/Analysis/analyzer-config.c:4-12
 void bar() {}
 void foo() {
   // Call bar 33 times so max-times-inline-large is met and
   // min-blocks-for-inline-large is checked
   for (int i = 0; i < 34; ++i) {
 bar();
   }

NoQ wrote:
> The code is no longer necessary, right? All options are now initialized 
> eagerly and there's no need to analyze any code to get them initialized.
Correct :)


Repository:
  rC Clang

https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: test/Analysis/analyzer-config.c:4-12
 void bar() {}
 void foo() {
   // Call bar 33 times so max-times-inline-large is met and
   // min-blocks-for-inline-large is checked
   for (int i = 0; i < 34; ++i) {
 bar();
   }

The code is no longer necessary, right? All options are now initialized eagerly 
and there's no need to analyze any code to get them initialized.



Comment at: test/Analysis/analyzer-config.cpp:1
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null 
-analyzer-checker=core,osx.cocoa,debug.ConfigDumper -analyzer-max-loop 34 > %t 
2>&1
 // RUN: FileCheck --input-file=%t %s --match-full-lines

Aaand this test should probably be removed entirely.


https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:227
-CTUDir = getStringOption("ctu-dir", "");
-if (!llvm::sys::fs::is_directory(*CTUDir))
-  CTUDir = "";

Szelethus wrote:
> This check should and will be moved to `parseConfigs` in a followup patch.
Just make sure to commit the patches together to not regress this check.


https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Also, thank you so much for the feedback on this, and almost every single other 
patch I made!


https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

In https://reviews.llvm.org/D53692#1285091, @NoQ wrote:

> I guess i'm running out of steam for today :)
>
> > `Opts.shouldDoThat()` -> `Opts.ShouldDoThat.getValue()`
>
> Is this really unavoidable? Like, what makes them optional when they're 
> always non-optional? Maybe just somehow prevent un-eagerly-initialized 
> AnalyzerConfig from appearing anywhere in the program?


Thats a fair point, the reason why I kept `Optional` is the extra safety -- 
it took a good couple hours to make this patch, and it didn't took days thanks 
to the `hasValue()` assert, since, well, how macros get expanded is always a 
surprise. But sure, ideally it shouldn't be there.


https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I guess i'm running out of steam for today :)

> `Opts.shouldDoThat()` -> `Opts.ShouldDoThat.getValue()`

Is this really unavoidable? Like, what makes them optional when they're always 
non-optional? Maybe just somehow prevent un-eagerly-initialized AnalyzerConfig 
from appearing anywhere in the program?


https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Polite ping. :)


https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

What are the feelings on moving some functions to the header file? That's the 
one thing I'm unsure about regarding this patch. I could've moved them to 
`CompilerInvocation.cpp`, but I since I plan on making `ConfigTable` private, 
`CompilerInvocation` needs to be a friend of `AnalyzerOptions`. Not that it's 
the end of the world, and similar classes (e.g.: `LangOptions`) work similarly.

The reason why I didn't do it straight away is that

- it's nice that `AnalyzerOptions` handles configs on it's own.
- if someone would like add a new config option and also add constraints (like 
it has to be an existing file) to that, facing the longer compilation times 
will be a burden anyways (because the .def file has to be modified), so I 
thought the maintenance cost of `parseConfig` won't be THAT bad,

but I'd concede to the fact that these aren't the greatest of concerns, though.


https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 171094.
Szelethus edited the summary of this revision.

https://reviews.llvm.org/D53692

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp

Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -19,6 +19,9 @@
 };
 
 // CHECK: [config]
+// CHECK-NEXT: aggressive-binary-operation-simplification = false
+// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
+// CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
@@ -32,6 +35,9 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: crosscheck-with-z3 = false
+// CHECK-NEXT: ctu-dir = ""
+// CHECK-NEXT: ctu-index-name = externalFnMap.txt
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
@@ -43,12 +49,22 @@
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: max-inlinable-size = 100
 // CHECK-NEXT: max-nodes = 225000
+// CHECK-NEXT: max-symbol-complexity = 35
 // CHECK-NEXT: max-times-inline-large = 32
 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
+// CHECK-NEXT: model-path = ""
+// CHECK-NEXT: notes-as-events = false
+// CHECK-NEXT: objc-inlining = true
+// CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2
+// CHECK-NEXT: report-in-main-source-file = false
 // CHECK-NEXT: serialize-stats = false
+// CHECK-NEXT: stable-report-filename = false
+// CHECK-NEXT: suppress-c++-stdlib = true
+// CHECK-NEXT: suppress-inlined-defensive-checks = true
+// CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 31
+// CHECK-NEXT: num-entries = 47
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -11,29 +11,52 @@
 }
 
 // CHECK: [config]
+// CHECK-NEXT: aggressive-binary-operation-simplification = false
+// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
+// CHECK-NEXT: c++-allocator-inlining = true
+// CHECK-NEXT: c++-container-inlining = false
+// 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-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: crosscheck-with-z3 = false
+// CHECK-NEXT: ctu-dir = ""
+// CHECK-NEXT: ctu-index-name = externalFnMap.txt
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
+// CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: inline-lambdas = true
 // CHECK-NEXT: ipa = dynamic-bifurcate
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: max-inlinable-size = 100
 // CHECK-NEXT: max-nodes = 225000
+// CHECK-NEXT: max-symbol-complexity = 35
 // CHECK-NEXT: max-times-inline-large = 32
 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
+// CHECK-NEXT: model-path = ""
+// CHECK-NEXT: notes-as-events = false
+// CHECK-NEXT: objc-inlining = true
+// CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2
+// 

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

The reason why I removed the getter functions and went ahead with the gigantic 
refactor is that getter functions actually changed the state of 
`AnalyzerOptions`, as they were responsible with the initialization of each 
option. Now, in the .def file, not all options had getter functions, so I 
couldn't just get away with calling every getter function once. Besides, a 
separate identifier for a field name (`NAME`) and getter function name 
(`CREATE_FN`) didn't make much sense anyways.




Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:227
-CTUDir = getStringOption("ctu-dir", "");
-if (!llvm::sys::fs::is_directory(*CTUDir))
-  CTUDir = "";

This check should and will be moved to `parseConfigs` in a followup patch.


Repository:
  rC Clang

https://reviews.llvm.org/D53692



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: xazax.hun, NoQ, george.karpenkov, MTC, rnkovacs.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, mgrang, szepet, whisperity.
Szelethus added a dependency: D53483: [analyzer] Restrict AnalyzerOptions' 
interface so that non-checker objects have to be registered.

This patch contains very little logic but touches a lot of lines.

Now that we're confident that //all// non-checker configs are gathered, I went 
ahead and did some actual changes to how `AnalyzerOptions` works.

This patch is a proposal to evaluate all config options (and possibly emit 
warning about them) at the beginning of the analysis, rather then whenever a 
getter function is called for it. This is done by changing the generated 
`Optional` fields' accessibility to public, evaluating all options as 
soon as the `ConfigTable` is assembled, and removing generated getters, as are 
they are no longer necessary. Now, every single public function (except 
`parseConfigs`) is const, as should `AnalyzerOptions` be after options are 
parsed (coming soon™ in a followup patch).

One important thing to notice is that the `ConfigTable` is no longer modified 
once received, so I needed to update `debug.ConfigDumper` accordingly.

The removal of those getters implied however that every single use of those 
getters had to be changed (`Opts.shouldDoThat()` -> 
`Opts.ShouldDoThat.getValue()`), and I got to simplify the def file. I 
clang-formatted it, and since I don't expect the format to change for a good 
long while, I think it should be clang-formatted in the future whenever.

Also coming soon™, the actual implementation of warnings.


Repository:
  rC Clang

https://reviews.llvm.org/D53692

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp

Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -19,6 +19,9 @@
 };
 
 // CHECK: [config]
+// CHECK-NEXT: aggressive-binary-operation-simplification = false
+// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
+// CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
@@ -32,6 +35,9 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: crosscheck-with-z3 = false
+// CHECK-NEXT: ctu-dir = ""
+// CHECK-NEXT: ctu-index-name = externalFnMap.txt
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
@@ -43,12 +49,22 @@
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: max-inlinable-size = 100
 // CHECK-NEXT: max-nodes = 225000
+// CHECK-NEXT: max-symbol-complexity = 35
 // CHECK-NEXT: max-times-inline-large = 32
 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
+// CHECK-NEXT: model-path = ""
+// CHECK-NEXT: notes-as-events = false
+// CHECK-NEXT: objc-inlining = true
+// CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2
+// CHECK-NEXT: report-in-main-source-file = false
 // CHECK-NEXT: serialize-stats = false
+// CHECK-NEXT: stable-report-filename = false
+// CHECK-NEXT: suppress-c++-stdlib = true
+// CHECK-NEXT: suppress-inlined-defensive-checks = true
+// CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 31
+// CHECK-NEXT: num-entries = 47
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -11,29 +11,52 @@
 }
 
 // CHECK: [config]
+// CHECK-NEXT: aggressive-binary-operation-simplification = false
+// CHECK-NEXT: