[clang] WIP: [clang] MicrosoftCXXABI: Serialize the exception copy constructor table in the AST (PR #114075)
Andrey Glebov via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 30 11:49:44 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:
Sorry, I haven't had as much time to work on this over the last few days.
But now I think I have a working version.
I created a serialization record with `CXXRecordDecl, CXXConstrctorDecl` pairs in it. It is written and read with all other record like `VTABLE_USES`, but only loaded into the AST on demand (is this correct?).
Generally some things seem dirty, like exposing the internal map in the interface. Also I'm not sure about naming - should MS ABI be mentioned everywhere, should it be called a "copy" constructor (even though that doesn't match the standard)?
> I'm proud of the depth of the test cases we added when implementing this the first time. There may be modules bugs, but at least there are good tests, and I remembered how to leverage them, so we don't have to rediscover all the edge cases. :)
Absolutely! Regarding tests, what kind of tests do we need for this feature? I assume something like `CodeGenCXX/microsoft-abi-throw.cpp`, but for deserialized ASTs? And there's probably something specific to AST reading/writing?
> I think we don't have to worry about cases involving deleted copy ctors because they will be diagnosed at the catch site (attempting to catch an uncopyable type by value), so the catchable type entry will effectively be dead.
I'm pretty sure there will still be a problem with `std::current_exception()` though - I'll have to test that.
https://github.com/llvm/llvm-project/pull/114075
More information about the cfe-commits
mailing list