================
@@ -13010,10 +13010,13 @@ static GVALinkage basicGVALinkageForFunction(const
ASTContext &Context,
if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
isa<CXXConstructorDecl>(FD) &&
- cast<CXXConstructorDecl>(FD)->isInheritingConstructor())
+ cast<CXXConstructorDecl>(FD)->isInheritingConstructor() &&
+ !FD->hasAttr<DLLExportAttr>())
// Our approach to inheriting constructors is fundamentally different from
// that used by the MS ABI, so keep our inheriting constructor thunks
// internal rather than trying to pick an unambiguous mangling for them.
+ // However, dllexport inherited constructors must be externally visible
+ // to match MSVC's behavior.
----------------
chinmaydd wrote:
Did some research and here are a couple of corner cases that may be relevant:
---
This PR exposes a bug with how parameters that need callee-cleanup, such as a
struct with a non-trivial destructor, are handled by clang-cl on i686. Example:
https://godbolt.org/z/EodccbW49.
This is also true for parameters that are declared `inalloca`. I believe these
were relevant to the changes referenced.
Specifically, for cases where
[canEmitDelegateCallArgs](https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGClass.cpp#L2338)
returns false we will take a different CodeGen path of
[inlining](https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGClass.cpp#L2392)
the inherited CXX calling constructor.
Currently, we are incompatible with MSVC because the internal linkage entirely
prevents the emission of the symbol for the inherited constructor even if it is
marked as dllexport. However, after merging this PR - we would still remain
incompatible. Except that the symbol is now exported, but is not ABI compatible
with MSVC. I could work on that as a follow-up or we could try and fix that as
part of this PR.
---
Fixed issue with `-fno-dllexport-inlines`. Inherited constructors are marked as
[inline](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclCXX.cpp#L14331)
which can prevent their symbol export.
---
Added a few test cases with default arguments and updated comments.
Curious to hear @zygoloid's insights.
https://github.com/llvm/llvm-project/pull/182706
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
