aaron.ballman added inline comments.
================ Comment at: clang/include/clang/Basic/AttrDocs.td:4167 + +The attribute ``[[gsl::Owner(T)]]`` applies to structs and classes that own an +object of type ``T``: ---------------- Do either of these attributes make sense on a union type? If so, might be worth mentioning unions here and below. If not, would it be worth diagnosing on a union? ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2516 "%0 and %1 attributes are not compatible">; +def err_attribute_invalid_argument : Error< + "%0 is an invalid argument to attribute %1">; ---------------- Can you combine this one with `err_attribute_argument_vec_type_hint`? ================ Comment at: clang/lib/Parse/ParseDecl.cpp:385 + TheParsedType = T.get(); + break; // Multiple type arguments are not implemented. + } else if (Tok.is(tok::identifier) && ---------------- Can you add a FIXME prefix to the comment? ================ Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4550 + S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument) + << "A reference type" << AL; + return; ---------------- This should say `a reference type`, but I prefer seeing this done in a `%select{}` within the diagnostic rather than manually adding string literals. This will also simplify the logic down to: ``` unsigned SelectIdx = ~0U; if (ParamType->isVoidType()) SelectIdx = 0; else if (ParamType->isReferenceType()) SelectIdx = 1; else if (ParamType->isArrayType()) SelectIdx = 2; if (SelectIdx != ~0U) { S.Diag(...) << SelectIdx << AL; return; } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits