[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

2019-12-23 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG379613d7c7fa: [CFG] Fix an assertion failure with static initializers (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D71791?vs=235161=235185#toc Repository: rG LLVM

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

2019-12-23 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. Woohoo tests!~~ Comment at: clang/unittests/Analysis/CFGBuildResult.h:69 + Finder.matchAST(AST->getASTContext()); + Callback.TheBuildResult.setAST(std::move(AST)); return std::move(Callback.TheBuildResult);

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

2019-12-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D71791#1795093 , @xazax.hun wrote: > I decided to fix the unittests. Having CFGs full of dangling pointers to the > AST does not look fun at

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

2019-12-23 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 235161. xazax.hun added a comment. I decided to fix the unittests. Having CFGs full of dangling pointers to the AST does not look fun at all, so I think this change was long overdue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71791/new/

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

2019-12-22 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Wait, i didn't real @Szelethus's reply :) In any case, i'd love to see an example of such CFG and whether //control dependency tracking// can expose the crash. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71791/new/

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

2019-12-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. > I am not sure what would be the best way to test this change here. A unittest, if you're brave :) Comment at: clang/lib/Analysis/CFG.cpp:5923 + if (isa(Cond) || isa(Cond)) return nullptr; Charusso wrote: > What about the

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

2019-12-21 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. > I am not sure what would be the best way to test this change here. I'll admit, I'm a bit out of touch with Clang and will unfortunately be for a while, but the short answer is that the unit tests we have for the CFG are kinda bad. The constructed CFG has a longer

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

2019-12-20 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:5923 + if (isa(Cond) || isa(Cond)) return nullptr; What about the following?: ```lang=c if (const auto *E = dyn_cast(StmtElem->getStmt())) return E->IgnoreParens(); return nullptr;

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

2019-12-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: Szelethus, NoQ. xazax.hun added a project: clang. Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, rnkovacs. The branches protecting the static initializers have a `DeclStmt` as last `Stmt`. Since it is not an