[PATCH] D155713: [clang] Fix interaction between dllimport and exclude_from_explicit_instantiation

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D155713#4592667 <https://reviews.llvm.org/D155713#4592667>, @rnk wrote:

>> That doesn't handle the second of your test cases though, where dllimport is put on the member directly:
>> ...
>> It's not clear to me how we'd want to handle that. I don't think it comes up in libc++, and I can't think of any code that would want to do that either, really.
>
> I think this does actually matter for libc++, I think I have seen this pattern:
>
>   template <typename T>
>   struct Foo {
>     _LIBCPP_FUNC_VIS _LIBCPP_EXCLUDE_FROM_ABI void method() { }
>   };
>
> I can't find an instance right now, though. I think it comes up when you want to have a default visibility function, and also keep it out of the libc++ DSO interface.

I guess the libc++ folks can answer if that's something that could occur.

> This is a somewhat weird and contradictory case, though, the attributes directly conflict. I think it would be reasonable to teach SemaDeclAttr to ignore the dllimport attribute if the other attribute is already present.

I think what worries me is at that point, the attribute is no longer about just suppressing explicit instantiation, but suppressing the dll attribute in general, which means perhaps it shouldn't just apply to templates, but also

  struct __declspec(dllimport)  S {
    void __attribute__((exclude_from_explicit_instantiation)) f() {}
  };

(This is what you mentioned in https://bugs.llvm.org/show_bug.cgi?id=41018#c1)

In one way I suppose it could make sense, but it's also pretty different from what the attribute is today, and I worry about growing its scope and the complexity that often comes with tweaking the behavior of the dll attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155713