wolfgangp added a comment.
In D145271#4171267 <https://reviews.llvm.org/D145271#4171267>, @hans wrote:
> Interesting! Do you have an example where this (local dllexport/import
> classes) comes up in practice?
A customer complained about the following code (I'm obscuring the class names)
compiling with MSVC but
rejected by clang:
template <class T>
class __declspec(dllimport) A
{
};
void func()
{
// local class with internal linkage
class B: public A<B>
{
};
}
The code derives from various instantiations of A in several contexts and it's
causing extra work for the customer to do something different in a local
context.
While I do agree that clang has it right I think this is more a case of
bug-compatibility with MSVC.
> MSVC will also allow the following:
>
> namespace {
> struct __declspec(dllexport) T {
> void foo();
> };
> }
>
> whereas Clang will not, and I think Clang is making the right call there.
Again, I do think clang makes the right call in all of these cases, but it's
about MSVC compatibility, for better or worse.
I did also look at the anonymous namespace case above, but that's a bit more
complicated, so I didn't address it this time.
================
Comment at: clang/lib/AST/Decl.cpp:492
+ // classes with dllexport/dllimport attributes.
+ if (spec->getASTContext().getTargetInfo().shouldDLLImportComdatSymbols())
+ if (spec->hasAttr<DLLExportAttr>() || spec->hasAttr<DLLImportAttr>())
----------------
hans wrote:
> We usually check ASTContext's `getTargetInfo().getCXXABI().isMicrosoft()`
That would not work for Playstation, since it's not following the Microsoft
ABI. The call to shouldDLLImport...(), checks the triple (it includes Windows
Itanium, Windows MSVC and PS).
Perhaps we need a less obscure predicate to indicate source compatibility with
MSVC as opposed to MS ABI compatibility.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145271/new/
https://reviews.llvm.org/D145271
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
