aaron.ballman added a comment.
I really like this attribute, thank you for working on this!
================
Comment at: clang/include/clang/Basic/Attr.td:2367
+def PreferredName : InheritableAttr {
+ let Spellings = [Clang<"preferred_name">];
+ let Subjects = SubjectList<[ClassTmpl]>;
----------------
This seems like one we should exempt from C code, WDYT? If you agree, you can
change it to `Clang<"preferred_name", /*AllowInC*/ 0>`
================
Comment at: clang/include/clang/Basic/Attr.td:2369
+ let Subjects = SubjectList<[ClassTmpl]>;
+ let Args = [TypeArgument<"TypedefType">];
+ let Documentation = [PreferredNameDocs];
----------------
Would it make sense for this to be a variadic parameter of type arguments (with
a constraint that at least one type be named)? This way you can write:
`[[clang::preferred_name(string, wstring)]]` instead of
`[[clang::preferred_name(string), clang::preferred_name(wstring)]]`
================
Comment at: clang/include/clang/Basic/AttrDocs.td:4398
+
+The preferred name must be a typedef declaration that refers to a
+specialization of the class template. In general this requires the template to
----------------
May want to be clear that this includes alias declarations as well as typedefs
(the example helps though). Also, you should mention that the typedef cannot
include qualifiers.
================
Comment at: clang/lib/AST/TypePrinter.cpp:1353
+ // Print the preferred name if we have one for this type.
+ for (PreferredNameAttr *PNA :
+ T->getDecl()->specific_attrs<PreferredNameAttr>()) {
----------------
`const auto *`?
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1394
+
+ if (!T->getAs<TypedefType>() || T.hasQualifiers()) {
+ S.Diag(AL.getLoc(), diag::err_attribute_preferred_name_arg_invalid)
----------------
Given that this is checking properties of the type used in the attribute, I
wonder if it makes sense to note the original declaration. I left a note on a
test case below about this.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:557
+ // template.
+ if (auto *PNA = dyn_cast<PreferredNameAttr>(A)) {
+ QualType T = PNA->getTypedefType();
----------------
Can you fix these two lint warnings?
================
Comment at: clang/test/SemaTemplate/attributes.cpp:79
+ template<typename T> struct [[clang::preferred_name(const X)]] C; //
expected-error {{argument 'const preferred_name::X'}}
+ template<typename T> struct [[clang::preferred_name(Z)]] C; //
expected-error {{argument 'preferred_name::Z' (aka 'const C<double>')}}
+ template<typename T> struct C {};
----------------
This looks like a pretty reasonable declaration but only fails because of an
issue with the declaration of `Z` -- should we point out more explicitly that
the qualifier on the alias declaration is the issue?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91311/new/
https://reviews.llvm.org/D91311
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
