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&id=133500#toc
Repositor
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:
https://reviews.llvm.org/
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
cfe-commits@lists.llvm.org
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 b
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 membe
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
include/clang/StaticAnalyzer/Core/AnalyzerOpti
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
d
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
include/clang/StaticAnalyz
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
> vi
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
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 already
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
include/clang/Analy
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 b
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
include/clang/StaticAnalyzer/Core/
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 CurrentConst
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
include/clang/Analysis/CFG.h
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 good
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
in
NoQ created this revision.
NoQ added reviewers: rsmith, dcoughlin, xazax.hun, a.sidorin, george.karpenkov,
szepet.
Herald added subscribers: cfe-commits, rnkovacs.
I'm planning to introduce some sort of `CFGConstructor` element, which would
help the analyzer (and otherwise whoever enables it - i
19 matches
Mail list logo