aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:2771
+ let Spellings = [CXX11<"gsl", "Owner">];
+ let Subjects = SubjectList<[CXXRecord]>;
+ let Args = [TypeArgument<"DerefType", /*opt=*/1>];
----------------
This subject should be `Struct` and same below.
================
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">;
----------------
mgehre wrote:
> aaron.ballman wrote:
> > Can you combine this one with `err_attribute_argument_vec_type_hint`?
> I'm not sure: vec_type_hint reads `"invalid attribute argument %0 - expecting
> a vector or vectorizable scalar type"` and here `""%0 is an invalid argument
> to attribute %1"`, i.e. one is positive ("expecting ...") and the other is
> negative ("%0 is an invalid argument").
>
> I don't know how to describe "not void, not reference, not array type" in
> terms of "expecting ...", and I think that we should keep "expecting a vector
> or vectorizable scalar type" on the VecTypeHint attribute diagnostic.
I had figured it would be something like `%select{'void'|a reference type|an
array type|a non-vector or non-vectorizable scalar type}0 is an invalid
argument to attribute %1` but I am not strongly opposed if you want to keep the
diagnostics separate.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2933
+ "|non-K&R-style functions"
+ "|classes}1">,
InGroup<IgnoredAttributes>;
----------------
You can drop this change when the `Subjects` field is fixed above.
================
Comment at: clang/include/clang/Sema/ParsedAttr.h:1037
ExpectedFunctionWithProtoType,
+ ExpectedClass,
};
----------------
Same
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4538-4542
+ if (!RD || RD->isUnion()) {
+ S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type)
+ << AL << ExpectedClass;
+ return;
+ }
----------------
You can drop this bit when the Subjects field is fixed above.
================
Comment at: clang/test/Misc/pragma-attribute-supported-attributes-list.test:119
// CHECK-NEXT: Overloadable (SubjectMatchRule_function)
+// CHECK-NEXT: Owner (SubjectMatchRule_record)
// CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
----------------
These will also wind up needing to be updated when switching the `Subjects`
field.
================
Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp:2
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int [[gsl::Owner]] i;
----------------
This file tests part of parsing and part of Sema somewhat interchangeably. I'm
not strictly opposed to it being that way, but it would be a bit cleaner to
have the parsing tests in the Parser directory and the rest in SemaCXX (that
also reduces the confusion from the file name).
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
