[PATCH] D145271: [MSVC compatibility][DLLEXPORT/DLLIMPORT] Allow dllexport/dllimport for local classes

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

In D145271#4178837 <https://reviews.llvm.org/D145271#4178837>, @wolfgangp wrote:

>> It still seems like the export/import there is an accident since `Base<f::Local>` can't really be referenced from outside the file anyway.
>>
>> Perhaps rather than giving `Base<f::Local>` external linkage to allow it to be imported/exported, the better fix would be to drop its dllimport/export attribute when instantiated with a local type?
>
> I like it. However, we would then not match MSVC's behavior in generating external definitions for the class (useless as they are). I suppose that's OK.
>
> But on another note, would we then abandon matching MSVC on the following?
>
>   void func() {
>     class __declspec(dllexport) A {
>     };
>   }
>
> This is currently accepted by MSVC, though arguably much less defensible than the above example.

Right. Unless someone shows a compelling example why it's needed, I think we should continue to reject that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145271/new/

https://reviews.llvm.org/D145271