rtrieu added a comment.
> I believe you were around this code last, so can you remember why it was
> there?
Yes, that's an early exit to speed up the check. You can remove that check and
add a test case for it.
This was a little confusing for me, so let's refactor it a little. These
changes will make it so that any CFGBlock in the WorkList has already been
checked, so it can go straight to checking the successors.
================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:203-204
-// All blocks are in one of three states. States are ordered so that blocks
-// can only move to higher states.
-enum RecursiveState {
- FoundNoPath,
- FoundPath,
- FoundPathWithNoRecursiveCall
-};
-
// Returns true if there exists a path to the exit block and every path
// to the exit block passes through a call to FD.
static bool checkForRecursiveFunctionCall(const FunctionDecl *FD, CFG *cfg) {
----------------
Update comment to reflect your changes.
================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:224-225
- // Mark all nodes as FoundNoPath, then set the status of the entry block.
- SmallVector<RecursiveState, 16> States(cfg->getNumBlockIDs(), FoundNoPath);
- States[cfg->getEntry().getBlockID()] = FoundPathWithNoRecursiveCall;
-
- // Make the processing stack and seed it with the entry block.
- SmallVector<CFGBlock *, 16> Stack;
- Stack.push_back(&cfg->getEntry());
-
- while (!Stack.empty()) {
- CFGBlock *CurBlock = Stack.back();
- Stack.pop_back();
+ // Seed the work list with the entry block.
+ foundRecursion |= analyzeSuccessor(&cfg->getEntry());
----------------
The entry CFGBlock is special. It will never trigger on any of the conditions
we are checking for. Just push it into WorkList and Visited. It has no
statements, so it can't have a recursive call.
http://clang.llvm.org/docs/InternalsManual.html#entry-and-exit-blocks
Technically, it has no incoming edges, so you don't even need to insert it into
Visited.
================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:232-233
+ // Found a path to the exit node without a recursive call.
+ if (ExitID == ID)
+ return false;
----------------
ID is not longer needed as an index so it is only needed here, so it should be
inlined.
Also, move this check in the loop over the successors, so there are no checks
on the current CFG block. This way, all checks happen at the same place.
================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:237
+ if (*I)
+ foundRecursion |= analyzeSuccessor(*I);
}
----------------
Since the lambda isn't needed for seeding the WorkList, this is the only use
and it should be inlined here.
Also, use:
```
if (cond)
foundRecursion = true;
```
instead of:
```
foundRecursion |= true;
```
https://reviews.llvm.org/D43737
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
