plotfi added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:2251-2256
+def ObjCDirectVisible : Attr {
+ let Spellings = [Clang<"objc_direct_visible">];
+ let Subjects = SubjectList<[ObjCMethod], ErrorDiag>;
+ let LangOpts = [ObjC];
+ let Documentation = [ObjCDirectVisibleDocs];
+}
----------------
mwyman wrote:
> Should this inherit `ObjCDirect`, to include both objc_direct and the
> visibility aspect? I don't see any reason we would want to add
> `objc_direct_visible` without also having `objc_direct`, so why make
> developers add both?
>
> As an alternative, would it make sense to allow adding
> `__attribute__((visibility("default")))` on direct methods?
>
> Also, it doesn't seem like this allows making `@property`s visible, so should
> there be a similar attribute for properties?
I'd prefer to do `@property`s in a separate commit, but I suppose you are
thinking like a `objc_direct_members_visible` attribute? I think I can add that
in a subsequent commit.
I took a look at how to make things inherit and I think the most
straightforward way is to have `handleObjCDirectVisibleAttr` set the
objc_direct attribute if it is not set already.
As for `__attribute__((visibility("default")))` I think the trouble lies in
what we want the default visibility behavior for objc methods to be and if we
want the behavior to be controlled by `-fvisibility=`. I tried going by
attribute visibility before and had some trouble too (I forget exactly what
though).
================
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4025-4036
+ if (OMD->isDirectMethodVisible() && Fn->getName().str()[0] == '\1') {
+ // Drop '\1' to work with dlsym.
+ std::string Name = Fn->getName().str().substr(1);
+
+ assert(Name[0] == '-' || Name[0] == '+');
+ assert(Name[1] == '[' && Name[Name.length() - 1] == ']');
+
----------------
mwyman wrote:
> Should this move to clang/lib/AST/Mangle.cpp's mangleObjCMethodName?
I like this idea. I will look into doing it next.
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
