[PATCH] D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods.

Fri Sep 23 12:23:34 PDT 2022

plotfi added a comment.

In D86049#3812068 <https://reviews.llvm.org/D86049#3812068>, @ahatanak wrote:

> In D86049#3806898 <https://reviews.llvm.org/D86049#3806898>, @plotfi wrote:
>
>> 1. Do we change the existing visibility behavior of objc methods? Yes / No
>
> I don't think we want to change the existing visibility behavior of non-direct objc methods. Is there a use reason or use case for making them visible outside the linkage unit?
>
>> 2. If we leave hidden as the default do we change the behavior for objc_direct? Yes / No
>
> I think direct methods shouldn't be hidden by default (i.e., they should get the default visibility). But it's not clear to me whether we should make that change right away as I've heard concerns from people internally. I think I need more time to understand what exactly their concerns are.
>
>> 3. If we leave objc_direct as hidden by default do we expand the existing objc_direct attr to have the enum as you said so `__attribute__((objc_direct("visible")))` or do we add a new attr as I have done so far?
>
> I wasn't sure why it wasn't possible to use the existing `__attribute__((visibility("default")))` attribute. Is it not possible to make only the direct methods get the default visibility?

This is not possible because default visibility is implicit (as far as I understand). It can not be checked if  `__attribute__((visibility("default")))` is set because it is always set unless -fvisibility=hidden is passed on the command line. So either we treat direct methods like everything else, and hide them when  `__attribute__((visibility("hidden")))` or the command line to hide everything by default is used, or we need a new attr or a new enum on the existing objc_direct attr.

Does this make sense or am I missing some details?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86049