[clang] WIP: [clang] MicrosoftCXXABI: Fix exception copy constructor LUT after loading AST (PR #114075)
Andrey Glebov via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 23 13:59:51 PST 2025
================
@@ -2316,6 +2316,20 @@ void ASTDeclReader::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
}
VisitCXXMethodDecl(D);
+
+ // Microsoft CXX ABI specific:
+ // Restore the RecordToCopyCtor sidecar LUT entry so that `throw` expressions
+ // find the correct copy constructor for exceptions during codegen.
+ // There is no need to check the target info because the
+ // HasCopyConstructorForExceptionObject bit only gets set for the MS ABI.
+ if (D->isCopyConstructor()) {
+ // TODO What if this is not the same copy constructor which was chosen by
+ // LookupCopyingConstructor() in SemaExprCXX? Is there a better way?
----------------
glebov-andrey wrote:
Well, ~~it worked~~... kind of, not really
```c++
#if 0
const CXXConstructorDecl *CD =
RD ? CGM.getContext().getCopyConstructorForExceptionObject(RD) : nullptr;
#else
const CXXConstructorDecl *CD = nullptr;
if (RD) {
for (const CXXConstructorDecl *Ctor : RD->ctors()) {
if (Ctor->isCopyConstructor()) {
// assert(!Ctor->isDeleted());
if (Ctor->isDeleted()) {
continue; // TODO why is this allowed? Should we break or continue?
}
if (!Ctor->isTrivial()) {
CD = Ctor;
}
break;
}
}
}
#endif
```
Simple cases worked fine, but a couple checks in `CodeGenCXX/microsoft-abi-throw.cpp` failed.
1. `@"_CT??_R0?AUDeletedCopy@@@81"`. As far as I can tell, it is not valid to throw a non-copyable type ([CWG-1863](https://cplusplus.github.io/CWG/issues/1863.html) and [CWG-2775](https://cplusplus.github.io/CWG/issues/2775.html)). This seems especially bad in the MS ABI where `std::exception_ptr` copies exceptions.
Anyway, just removed my assertion that the copy constructor wasn't deleted, and it passed.
2. `@"_CT??_R0?AUTemplateWithDefault@@@8??$?_OH at TemplateWithDefault@@QAEXAAU0@@Z1"`
This one I'm not so sure about. The resolution for [CWG-2775](https://cplusplus.github.io/CWG/issues/2775.html) doesn't appear to actually require a _copy constructor_, or for the source object to be considered as a _const_ lvalue.
If I'm understanding this correctly then this in itself looks like a defect (but that's beyond the point, and probably an ABI issue).
As it turns out, only the const-param constructor is identified as `isCopyConstructor() == true`. From my reading of `[class.copy]` this is because the constructor template is not a _copy constructor_ but can be used for _copy-initialization_. Presumably this is the reason we need the lookup table based on proper overload resolution.
It seems that after all we do need to explicitly store the lookup table in the AST, or am I missing something?
https://github.com/llvm/llvm-project/pull/114075
More information about the cfe-commits
mailing list