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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
