[libcxx-commits] [PATCH] D97168: [libcxx] [test] Add -Wno-dllimport-static-field-def when building tests
Martin Storsjö via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Feb 22 14:26:11 PST 2021
mstorsjo added a comment.
In D97168#2579763 <https://reviews.llvm.org/D97168#2579763>, @rnk wrote:
> In D97168#2579077 <https://reviews.llvm.org/D97168#2579077>, @ldionne wrote:
>
>> Is this really the right fix? I think Clang is warning about an actual issue, i.e. that we're defining `npos` in the test file when we shouldn't be? If we just silence this warning in the test suite, it'll fire when people build against libc++ on Windows.
>>
>> What's the Microsoft recommendation for using `dllimport` on static data members?
>
> I guess MSVC is more strict, they just reject dllimport when applied to a data definition:
> https://gcc.godbolt.org/z/Mqs113
> I can't recall why we made this a warning in clang instead of an error.
>
> It seems wrong to use the `_LIBCPP_FUNC_VIS` attribute on things that are not functions, so maybe we need a `_LIBCPP_DATA_VIS` attribute.
Thanks! A `_LIBCPP_DATA_VIS` sounds like a good way forward to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97168/new/
https://reviews.llvm.org/D97168
More information about the libcxx-commits
mailing list