rnk added a comment.
I like the implementation strategy, but I have one corner case, which is
probably worth a test.
================
Comment at: clang/lib/CodeGen/CGCleanup.cpp:848
// Build a switch-out if we need it:
// - if there are branch-afters threaded through the scope
----------------
Are we sure this is the right set of predicates? It seems like we'll get the
wrong flag in cases like:
```
__try {
mayThrow();
goto out; // AbnormalTermination should be 1
} __finally {
tearDown();
}
out:
return 0;
```
I think these conditions are designed to avoid the switch when there is no
normal fallthrough destination (!HasFallthrough). I see clang emits no switch
for this C++ input:
```
$ cat t.cpp
void mayThrow();
struct Dtor {
~Dtor();
};
int f() {
do {
Dtor o;
mayThrow();
break;
} while (0);
return 0;
}
$ clang t.cpp -S -emit-llvm -fno-exceptions -o - | grep switch
# empty
```
================
Comment at: clang/lib/CodeGen/CGCleanup.cpp:863-864
+ // pass the abnormal exit flag to Fn (SEH cleanup)
+ cleanupFlags.setHasSehAbnormalExits();
+
----------------
This flag is set for any cleanup that needs to switch-out to multiple normal
destinations. You have named this an "seh abnormal exit", an exit that is
abnormal for the purposes of SEH, but I think it would be clearer to give the
flag a more general name.
Based on the existing terminology, maybe (F_)HasBranchAfter would be the
closest to what we are looking for?
================
Comment at: clang/lib/CodeGen/EHScopeStack.h:184
+ bool hasSehAbnormalExits() const { return flags & F_HasSehAbnormalExits;
}
+ void setHasSehAbnormalExits() { flags |= F_HasSehAbnormalExits; }
----------------
Please add a comment describing the accessor.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77936/new/
https://reviews.llvm.org/D77936
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
