philnik777 wrote:
> > I too had trouble understanding this change based on the description. Could
> > you give a concrete example of how mingw and msvc disagree on where to put
> > the attribute, and explain how this pr changes things?
```
template <class>
class S {};
extern template class __declspec(dllexport) S<short>; // msvc complains about
this __declspec
template class __declspec(dllexport) S<short>; // mingw complains about
this __declspec
```
With this PR Clang doesn't complain in either mode if `__declspec` is on both.
Does that make it clear?
> > Taking a step back, how will this simplify libc++'s visibility annotations?
> > Don't they still need to be compatible with both mingw and msvc, which we
> > don't control?
>
> Can you respond to these questions here, they're essential to the motivation
> of this change IMO.
>
> Also, won't the potential simplification in libc++ essentially just amount to
> this?
>
> ```diff
> diff --git a/libcxx/include/__config b/libcxx/include/__config
> index 38c47e8d45c8..4102e41ebe23 100644
> --- a/libcxx/include/__config
> +++ b/libcxx/include/__config
> @@ -341,13 +341,8 @@ typedef __char32_t char32_t;
> # define _LIBCPP_OVERRIDABLE_FUNC_VIS
> # define _LIBCPP_EXPORTED_FROM_ABI
> # elif defined(_LIBCPP_BUILDING_LIBRARY)
> -# if defined(__MINGW32__)
> -# define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS __declspec(dllexport)
> -# define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
> -# else
> -# define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
> -# define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
> __declspec(dllexport)
> -# endif
> +# define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS __declspec(dllexport)
> +# define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS __declspec(dllexport)
> # define _LIBCPP_OVERRIDABLE_FUNC_VIS __declspec(dllexport)
> # define _LIBCPP_EXPORTED_FROM_ABI __declspec(dllexport)
> # else
> ```
No. We can drop `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` and
`_LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS` by replacing it with
`_LIBCPP_EXPORTED_FROM_ABI`. The simplification is that we have a single macro
to say that something is in the dylib. Having to explain that mingw and msvc
don't like each others annotation placements is much more complicated than
saying we require `_LIBCPP_EXPORTED_FROM_ABI` on both the extern declaration
and definition.
> The downside is that it would be _massively_ much more confusing to use
> Clang, as a developer, if it silently accepts `dllexport` in the wrong place,
> if it just doesn't do anything with it there.
> As a developer, you'd have to essentially guess and try blindly until you
> stick it in the right place where it actually does the right thing, if Clang
> doesn't tell you about cases where it's silently ignored.
Clang will only accept an incorrect placement silently if there is a correct
annotation as well. If it is actually ignored there will be a diagnostic.
https://github.com/llvm/llvm-project/pull/133699
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
