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

2023-03-08 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. In D145271#4178837 , @wolfgangp wrote: >> It still seems like the export/import there is an accident since >> `Base` can't really be referenced from outside the file anyway. >> >> Perhaps rather than giving `Base` external linkage

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

2023-03-08 Thread Wolfgang Pieb via Phabricator via cfe-commits
wolfgangp added a comment. > It still seems like the export/import there is an accident since > `Base` can't really be referenced from outside the file anyway. > > Perhaps rather than giving `Base` external linkage to allow it to > be imported/exported, the better fix would be to drop its

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

2023-03-08 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a reviewer: mstorsjo. hans added a comment. +mstorsjo for thoughts about Windows code, even if this might not apply to mingw. In D145271#4172636 , @wolfgangp wrote: > A customer complained about the following code (I'm obscuring the class >

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

2023-03-06 Thread Wolfgang Pieb via Phabricator via cfe-commits
wolfgangp added a comment. In 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)

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

2023-03-06 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. Interesting! Do you have an example where this (local dllexport/import classes) comes up in practice? MSVC will also allow the following: namespace { struct __declspec(dllexport) T { void foo(); }; } whereas Clang will not, and I think Clang is making the

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

2023-03-03 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a subscriber: cfe-commits. probinson added a project: clang. probinson added a comment. I've looked at this but I'd like someone more in tune with MSVC behavior to review as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145271/new/