[PATCH] D113167: [lld-macho] Allow exporting weak def private-extern symbols.

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added a comment.

In D113167#3112543 <https://reviews.llvm.org/D113167#3112543>, @thakis wrote:

> In D113167#3109836 <https://reviews.llvm.org/D113167#3109836>, @oontvoo wrote:
>
>> In D113167#3109188 <https://reviews.llvm.org/D113167#3109188>, @thakis wrote:
>>
>>> That honestly sounds like a bug in your build config, independently of if ld64 allows it, no?
>>
>> I still think that LLD  shouldn't conflate the following two cases:
>>
>>   // hidden.cc
>>   extern __attribute__((weak)) __attribute__((visibility("hidden"))) void foo()  {return 5;}
>>   int bar() { return foo(); }
>>
>> VS
>>
>>   // can_be_hidden.cc (Must be C++, not C)
>>   __attribute__((noinline)) inline int foo() { return 5; }
>>   int bar() { return foo(); }
>>
>> For the `hidden.cc`, clearly we shouldn't export `foo`. (current behaviour).
>> But for the latter, it should be fine
>
> You can make a case for this, but as far as I understand the only programs where this can make a difference in practice is one with ODR violations, right?

If the copy is private, then I don't think it's strictly ODR violations.
But maybe two orthogonal points here:

- is it correct to make autohide symbols synonym with private-extern? (agreed that it likely has no observable difference in the output)
- is it correct to prevent autohide symbols from being exported?  (From the  example, there's no reason `foo` shouldn't be exportable - it's a bit weird, yes, but it's not wrong)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113167