[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

This revision was automatically updated to reflect the committed changes. Closed by commit rC324668: [CFG] Add extra context to C++ constructor statement elements. (authored by dergachev, committed by ). Changed prior to commit: https://reviews.llvm.org/D42672?vs=133139=133500#toc Repository:

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

This revision was automatically updated to reflect the committed changes. Closed by commit rL324668: [CFG] Add extra context to C++ constructor statement elements. (authored by dergachev, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ added a comment. All right, so this change is indeed safe, i.e. no crashes or changes in analyzer behavior so far on my large codebase run. Will commit now. https://reviews.llvm.org/D42672 ___ cfe-commits mailing list

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ updated this revision to Diff 133139. NoQ added a comment. Make CFGConstructor a sub-class of CFGStmt. I failed to notice any compile time regressions for now; if any, they must be super bottlenecked on CFG construction or analysis-based warnings. After poking @rsmith in the chat a little

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ added inline comments. Comment at: include/clang/Analysis/CFG.h:153 + + ConstructionContext() = default; + ConstructionContext(CXXConstructExpr *Constructor, Stmt *Trigger) xazax.hun wrote: > Maybe I am getting this wrong, but I think in this case the

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ updated this revision to Diff 132908. NoQ marked an inline comment as done. NoQ added a comment. Fix uninitialized variables. https://reviews.llvm.org/D42672 Files: include/clang/Analysis/AnalysisDeclContext.h include/clang/Analysis/CFG.h

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

xazax.hun added inline comments. Comment at: include/clang/Analysis/CFG.h:153 + + ConstructionContext() = default; + ConstructionContext(CXXConstructExpr *Constructor, Stmt *Trigger) Maybe I am getting this wrong, but I think in this case the members will be

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ updated this revision to Diff 132685. NoQ added a comment. Rename the method, add comments. https://reviews.llvm.org/D42672 Files: include/clang/Analysis/AnalysisDeclContext.h include/clang/Analysis/CFG.h include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ added inline comments. Comment at: lib/Analysis/CFG.cpp:653 + // triggered by the given trigger. + void VisitForConstructionContext(Stmt *Trigger, Stmt *Child); + dcoughlin wrote: > I find the name of this method very confusing. It sounds like you are >

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. This looks good to me, although as I noted inline I think both the name and the comment for VisitForConstructionContext() are confusing. If you can be more precise I think it would help

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ added inline comments. Comment at: lib/Analysis/CFG.cpp:3899 + if (auto *CE = const_cast(NE->getConstructExpr())) +CurrentConstructionContext = {/*Constructor=*/CE, /*Trigger=*/NE}; + NoQ wrote: > dcoughlin wrote: > > Is it possible that there is

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ updated this revision to Diff 132637. NoQ added a comment. Whoops! Actually use the new function with the assertion. Also disable creating construction contexts when the flag is off. https://reviews.llvm.org/D42672 Files: include/clang/Analysis/AnalysisDeclContext.h

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ updated this revision to Diff 132532. NoQ marked an inline comment as done. NoQ added a comment. Cleanup construction contexts after they have been consumed. Add assertions to verify that our understanding of the context's lifespan is reasonable. I will run the new mode on a large codebase

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ updated this revision to Diff 132520. NoQ added a comment. Add more targeted tests. https://reviews.llvm.org/D42672 Files: include/clang/Analysis/AnalysisDeclContext.h include/clang/Analysis/CFG.h include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ added inline comments. Comment at: lib/Analysis/CFG.cpp:3899 + if (auto *CE = const_cast(NE->getConstructExpr())) +CurrentConstructionContext = {/*Constructor=*/CE, /*Trigger=*/NE}; + dcoughlin wrote: > Is it possible that there is already a

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ updated this revision to Diff 132476. NoQ marked 5 inline comments as done. NoQ added a comment. Address comments. Most importantly, separate out sema and analyzer CFG tests. https://reviews.llvm.org/D42672 Files: include/clang/Analysis/AnalysisDeclContext.h

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

dcoughlin added a comment. I think it will be great to have a more explicit representation of construction in the CFG! Comments in line. Comment at: include/clang/Analysis/CFG.h:143 + CXXConstructExpr *Constructor; + Stmt *Trigger; + I think it would be

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ updated this revision to Diff 132053. NoQ added a comment. Remove the stack of contexts for now. We're not using it anywhere yet, and i'm not sure if it'd be necessary. https://reviews.llvm.org/D42672 Files: include/clang/Analysis/AnalysisDeclContext.h include/clang/Analysis/CFG.h