-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang][cleanup] simplify ReachableCode scanFromBlock #72257
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: thyecust (thyecust) ChangesWe can simplify this function and move some calculations out of loop. Full diff: https://github.com/llvm/llvm-project/pull/72257.diff 1 Files Affected:
diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index 1bf0d9aec8620e1..9c9e1a0fffb3c98 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -341,30 +341,27 @@ static unsigned scanFromBlock(const CFGBlock *Start,
// This allows us to potentially uncover some "always unreachable" code
// within the "sometimes unreachable" code.
// Look at the successors and mark then reachable.
- std::optional<bool> TreatAllSuccessorsAsReachable;
- if (!IncludeSometimesUnreachableEdges)
+ bool TreatAllSuccessorsAsReachable;
+ if (IncludeSometimesUnreachableEdges) {
+ assert(PP);
+ TreatAllSuccessorsAsReachable =
+ shouldTreatSuccessorsAsReachable(item, *PP);
+ } else {
TreatAllSuccessorsAsReachable = false;
+ }
for (CFGBlock::const_succ_iterator I = item->succ_begin(),
E = item->succ_end(); I != E; ++I) {
const CFGBlock *B = *I;
- if (!B) do {
+ if (!B) {
const CFGBlock *UB = I->getPossiblyUnreachableBlock();
if (!UB)
break;
- if (!TreatAllSuccessorsAsReachable) {
- assert(PP);
- TreatAllSuccessorsAsReachable =
- shouldTreatSuccessorsAsReachable(item, *PP);
- }
-
- if (*TreatAllSuccessorsAsReachable) {
+ if (TreatAllSuccessorsAsReachable) {
B = UB;
- break;
}
}
- while (false);
if (B) {
unsigned blockID = B->getBlockID();
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
clang/lib/Analysis/ReachableCode.cpp
Outdated
|
||
for (CFGBlock::const_succ_iterator I = item->succ_begin(), | ||
E = item->succ_end(); I != E; ++I) { | ||
const CFGBlock *B = *I; | ||
if (!B) do { | ||
if (!B) { | ||
const CFGBlock *UB = I->getPossiblyUnreachableBlock(); | ||
if (!UB) | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a functional change. Previously this break was breaking out of the do-while
loop, now it is breaking out of the for
loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right. I should replace this break with continue.
Because previously this break, under if (!B) {
, was going to if (B) {
and contining the next for
loop iteration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for review. The fix is done.
We can simplify this function and move some calculations out of loop.