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]; +} ---------------- 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. 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