xazax.hun added inline comments.
================
Comment at: clang/lib/Analysis/CFG.cpp:5619-5625
+ // If the terminator is a temporary dtor or a virtual base, etc, we can't
+ // retrieve a meaningful condition, bail out.
+ if (rbegin()->getKind() != CFGElement::Kind::Statement)
+ return nullptr;
+
+ // This should be the condition of the terminator block.
+ const Stmt *S = rbegin()->castAs<CFGStmt>().getStmt();
----------------
NoQ wrote:
> I don't think you're looking at the terminator here, as `CFGTerminator` isn't
> a sub-class of `CFGElement`. So this if doesn't avoid temporary dtor branches
> or virtual base branches; instead, it avoids various other `CFGElement`
> sub-classes such as `CFGInitializer` or `CFGImplicitDtor` (not sure how many
> of those may appear last in a block).
>
> Therefore the following code would be a bit more LLVM-y:
> ```lang=c++
> auto StmtElem = rbegin()->getAs<CFGStmt>();
> if (!StmtElem)
> return nullptr;
>
> const Stmt *S = StmtElem->getStmt();
> ```
>
> Also, are you sure that the last expression is always a condition? Like, what
> about
> ```lang=c++
> void foo(int x, int y) {
> (void)(x + y);
> }
> ```
> ?
The question is, is `CFGTerminator ` is what we are looking for? I think the
point of the method is to get the last subexpression that was evaluated before
taking a branch.
For the condition, would it make sense to check the number of successors
(before pruning the trivially false branches)? If less than or equal to 1 it is
probably not a condition. Otherwise maybe we could just rename it to last
evaluated (sub)expression. The sub-part might depend on whether we use a
linearized CFG.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63538/new/
https://reviews.llvm.org/D63538
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
