ahatanak added a comment. In D86049#3818696 <https://reviews.llvm.org/D86049#3818696>, @plotfi wrote:
> In D86049#3818435 <https://reviews.llvm.org/D86049#3818435>, @mwyman wrote: > >> In D86049#3816006 <https://reviews.llvm.org/D86049#3816006>, @plotfi wrote: >> >>> @dmaclach @ahatanak @mwyman How do things look from here? Do you want >>> something for properties as well or would it be ok if we did this in a >>> later commit? >> >> Huh, for some reason I thought when I'd last poked at using the `visibility` >> attribute it wasn't allowed on ObjC methods, which is why I'd suggested >> adding the enum on `objc_direct`, but as that no longer appears to be the >> case yes I like this approach better. > > @dmaclach @mwyman I am also very happy with the fact that we can just reuse > the regular `visibility` attribute. In the future we can decide on revised > behavior for `hasMethodVisibilityDefault`. > > @ahatanak Do you have feedback on this? The visibility attribute changes look good to me. Do we have the answers to the last two questions John raised in https://reviews.llvm.org/D86049#2255738? I think we should get it right the first time since, once we make the direct methods visible, it'd be hard to change where the null check or class initialization is done since that would be an ABI change. Should we run experiments to measure the impact on code size? ================ Comment at: clang/lib/AST/Mangle.cpp:371 + OS << (MD->isInstanceMethod() ? '-' : '+'); + OS << (MD->hasMethodVisibilityDefault() ? '<' : '['); if (const auto *CID = MD->getCategory()) { ---------------- Sorry, I might have missed the discussion, but what's the reason we need this change in mangling? Is it because the linker cannot handle the standard mangling scheme? 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
