plotfi marked an inline comment as done.
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];
+}
----------------
plotfi wrote:
> 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). 
> 
> 
I gave visibility a try and it seems that the trouble is everything is visible 
by default where for objc methods we want them hidden by default. I think I 
would rather add a separate attr for this than add an additional non-conformant 
visibility mode. 


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

Reply via email to