aaron.ballman added a comment. In D84005#2171962 <https://reviews.llvm.org/D84005#2171962>, @MForster wrote:
> In D84005#2162433 <https://reviews.llvm.org/D84005#2162433>, @aaron.ballman > wrote: > > > It's a bit odd that this attribute has an AST node created for it but > > nothing is using that AST node elsewhere in the project. Are there other > > patches expected for making use of this attribute? > > > It gets used from Swift. Are there plans to use this attribute in Clang itself? If not, is there a way Swift can use attribute plugins instead of modifying Clang? ================ Comment at: clang/include/clang/Basic/AttrDocs.td:3330 + +For example: + ---------------- MForster wrote: > aaron.ballman wrote: > > I still feel like I'm lacking information about the domain. In the example, > > the domain is an uninitialized `const char *`, so how does this information > > get used by the attribute? Can I use any type of global I want? What about > > a literal? > I won't claim that I understand this well. Maybe @gribozavr2 or @milseman can > explain this better. Literals are not allowed, however. The test explicitly > forbids that. > Literals are not allowed, however. The test explicitly forbids that. And I'm wondering why. For instance, attributes with enumeration arguments accept the enumerator as either an identifier or a string literal. I was sort of assuming that these domains are notionally like enumerators in that there is a list of domains, which is why I was asking about literals. ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9454 + "domain argument %0 does not refer to global constant">; +def err_nserrordomain_requires_identifier : Error< + "domain argument must be an identifier">; ---------------- MForster wrote: > aaron.ballman wrote: > > I don't think this requires a custom diagnostic -- we can use > > `err_attribute_argument_n_type` instead. > > I don't think this requires a custom diagnostic -- we can use > > `err_attribute_argument_n_type` instead. > > > Done. Can you please confirm that `n` is 1-based? Yes, the argument positions are 1-based. ================ Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5336 + } + IdentifierLoc *identLoc = + Attr.isArgIdent(0) ? Attr.getArgAsIdent(0) : nullptr; ---------------- aaron.ballman wrote: > Variables should match the usual coding style conventions. It looks like `loc` was missed and still need to be updated to `Loc`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84005/new/ https://reviews.llvm.org/D84005 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
