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:
> > plotfi wrote:
> > > 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. 
> > Re: visibility, I wonder if it might make sense to create an optional enum 
> > argument on the `objc_direct` and `objc_direct_members` attributes, with 
> > either `hidden` or `visible` values (and presumably `hidden` being 
> > default); if we have an `objc_direct_members_visible`-like attribute, would 
> > there be cases where someone may wish to hide individual members?
> > 
> > This is quite possibly over-thinking the issue, but it also then avoids 
> > having an entirely new pair of method attributes. It doesn't solve the 
> > `@property` attributes, which don't have arguments, but it may be 
> > unavoidable to add a completely new attribute for that.
> Ah so something like `__attribute__((objc_direct("default")))` or  
> `__attribute__((objc_direct("visible")))` then? Hmm I wonder if that could be 
> just what we want, that actually sounds pretty good.
> Ah so something like `__attribute__((objc_direct("default")))` or  
> `__attribute__((objc_direct("visible")))` then? Hmm I wonder if that could be 
> just what we want, that actually sounds pretty good.

@mwyman One thing I am not sure of is, is it possible to add an enum argument 
to something like `__atttribute__(objc_direct)` while providing a default enum 
argument so that the visibility string doesn't always have to be specified. I 
have not seen a version of this 


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