[libcxx-commits] [PATCH] D97168: [libcxx] Don't use dllimport for a static member in a template

Tue Mar 2 08:30:11 PST 2021

mstorsjo added inline comments.

================
Comment at: libcxx/include/__config:673

+#ifndef _LIBCPP_DATA_VIS
+#  if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
----------------
rnk wrote:
> mstorsjo wrote:
> > ldionne wrote:
> > > Maybe this can just be called `_LIBCPP_DEFAULT_VISIBILITY`?
> > > 
> > > And we could maybe also reuse this macro in the various `_LIBCPP_FUNC_VIS` & other macros (as a separate patch, I can try to clean this up).
> > Hmm, not entirely sure, that changes the semantics a bit: Currently the attributes say what they're supposed to annotate (function visibility, type visibility, etc), but that'd make it say what it does (on one platform) - which actually isn't the same across all platforms (the point here being we expressly want a different visibility for windows targets, for static members in a template, contrary to the other cases).
> Yeah, this is tough. Seeing the new name, I'm not sure I like it. @ldionne is right, we can export data. The restriction is that things with weak linkage can't be imported/exported, since they don't belong only to the libc++ DLL.
> 
> Do you think it's reasonable to use `_LIBCPP_TEMPLATE_VIS`? `std::basic_string<T...>::npos` is a "templated entitiy" (temploid?). It should work as is.
`_LIBCPP_TEMPLATE_VIS` seems ok to me; a new `_LIBCPP_TEMPLATE_DATA_VIS` would work too if we'd want a more precise name.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97168