MaskRay added a comment.
In D133266#3775189 <https://reviews.llvm.org/D133266#3775189>, @bd1976llvm
wrote:
> This approach doesn't account for the implementation of
> -fvisibility-global-new-delete-hidden. The following case now fails after
> this change:
>
> >type new_del.cpp
>
> namespace std {
> typedef __typeof__(sizeof(int)) size_t;
> struct nothrow_t {};
> struct align_val_t {};
> }
> __declspec(dllexport) void operator delete(void* ptr) throw() {return;}
>
>
> >clang.exe --target=x86_64-pc-windows-gnu new_del.cpp
> -fvisibility-global-new-delete-hidden -c
> new_del.cpp:8:28: error: non-default visibility cannot be applied to
> 'dllexport' declaration
> __declspec(dllexport) void operator delete(void* ptr) throw() {return;}
> ^
> 1 error generated.
From a glance, `-fvisibility-global-new-delete-hidden` should make the
visibility implicit (so won't trigger this error) but don't seem to do so? I'll
investigate later.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1104
if (GV->hasLocalLinkage()) {
GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
return;
----------------
mstorsjo wrote:
> I was wondering if we're changing behaviour in some other case since this is
> now checked before the dllexport/dllimport, but I think it shouldn't make any
> difference, since we already error out (elsewhere) on dllexport+static today.
Confirmed. We have diagnostics like:
```
a.c:1:35: error: 'f' must have external linkage when declared 'dllexport'
static __declspec(dllexport) void f() {}
^
a.c:2:35: error: 'g' must have external linkage when declared 'dllimport'
static __declspec(dllimport) void g() {}
^
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133266/new/
https://reviews.llvm.org/D133266
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
