[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-29 Thread Wolfgang Pieb via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG477f9cea77e6: [MSCV][dllexport/dllimport][PS] Allow UniqueExternal linkage classes with… (authored by wolfgangp). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-28 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146338/new/ https://reviews.llvm.org/D146338 ___ cfe-commits mailing list

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-27 Thread Wolfgang Pieb via Phabricator via cfe-commits
wolfgangp added inline comments. Comment at: clang/test/SemaCXX/dllexport.cpp:437 +class Base {}; +class __declspec(dllexport) ExportedClass {}; hans wrote: > Is this one used somewhere? It's not. Thanks for finding it. Comment at:

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-27 Thread Wolfgang Pieb via Phabricator via cfe-commits
wolfgangp updated this revision to Diff 508747. wolfgangp marked an inline comment as done. wolfgangp added a comment. Added 2 test cases that check that dll{ex,im}ported classes that are instantiated with classes with internal linkage as template arguments are not exported or imported. Had to

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-23 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. Thanks, I like this. Could you please also add tests in CodeGenCXX/dll{ex,im}port.cpp which verify that the IR looks right? Comment at: clang/test/SemaCXX/dllexport.cpp:437 +class Base {}; +class __declspec(dllexport) ExportedClass {};

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-22 Thread Wolfgang Pieb via Phabricator via cfe-commits
wolfgangp updated this revision to Diff 507428. wolfgangp added a comment. Instead of suppressing the error message we drop the dllimport/dllexport attribute when we see that a class has UniqueExternal linkage. This will suppress exporting or importing any members, which could not be accessed

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-22 Thread Wolfgang Pieb via Phabricator via cfe-commits
wolfgangp added a comment. In D146338#4206222 , @hans wrote: >> In D145271 it was suggested that we drop >> the attribute in such contexts, and this is effectively what happens. The >> compiler does not produce any

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-20 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. > In D145271 it was suggested that we drop > the attribute in such contexts, and this is effectively what happens. The > compiler does not produce any exported definitions (or import any symbols) > for such classes. The patch is simply

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-17 Thread Wolfgang Pieb via Phabricator via cfe-commits
wolfgangp created this revision. wolfgangp added reviewers: hans, probinson, mstorsjo. Herald added a project: All. wolfgangp requested review of this revision. This replaces D145271 . Rather than coercing classes with UniqueExternalLInkage to ExternalLinkage as