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

Puyan Lotfi via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 26 06:44:27 PDT 2022


plotfi added a comment.

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

> In D86049#3812412 <https://reviews.llvm.org/D86049#3812412>, @plotfi wrote:
>
>> 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?
>
> But there are ways to check whether the user explicitly added a visibility attribute (e.g., `__attribute__((visibility("default")))`), right?  For example, `NamedDecl::getExplicitVisibility`.
>
> I'm just not sure whether we want to add support for a new attribute like `__attribute__((objc_direct("default")))` since it seems equivalent to `__attribute__((objc_direct, visibility("default")))`.

Ah, thank you Akira. I had tried to explicitly check for this in some ways but not through `NamedDecl::getExplicitVisibility`. I will give this a try and if this works I think we can move forward.

Thanks Again

Puyan


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86049



More information about the cfe-commits mailing list