aaron.ballman added inline comments.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2068
@@ -2064,1 +2067,3 @@
def err_attribute_pointers_only : Error<warn_attribute_pointers_only.Text>;
+def err_attribute_constant_pointers_only : Error<
+ "%0 attribute only applies to constant pointer arguments">;
----------------
Instead of making a new diagnostic, I think it better to modify
warn_attribute_pointers_only to use a %select statement (then
err_attribute_pointers_only also gets updated). I would separate that into a
prequel patch since it's likely to introduce a fair amount of unrelated changes
to this patch.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:820
@@ +819,3 @@
+ Expr *E = Attr.getArgAsExpr(0);
+ IntegerLiteral *Int = dyn_cast<IntegerLiteral>(E->IgnoreParenCasts());
+ if (Int == nullptr) {
----------------
We don't usually ignore parens and casts for attributes like this for other
attributes. Is that important for you uses for some reason?
================
Comment at: lib/Sema/SemaDeclAttr.cpp:828
@@ +827,3 @@
+ auto APType = Int->getValue();
+ if (APType.ugt(3) || APType.slt(0)) {
+ S.Diag(E->getLocStart(), diag::err_attribute_argument_out_of_bounds)
----------------
rsmith wrote:
> The second check here is redundant if `APType` is more than 2 bits wide (and
> looks wrong when `Int` is of type `bool`).
I'm wondering why the magic value 3 at all? It seems like this (and the above
code) should be replaced by a call to checkFunctionOrMethodParameterIndex().
================
Comment at: lib/Sema/SemaDeclAttr.cpp:828
@@ +827,3 @@
+ auto APType = Int->getValue();
+ if (APType.ugt(3) || APType.slt(0)) {
+ S.Diag(E->getLocStart(), diag::err_attribute_argument_out_of_bounds)
----------------
aaron.ballman wrote:
> rsmith wrote:
> > The second check here is redundant if `APType` is more than 2 bits wide
> > (and looks wrong when `Int` is of type `bool`).
> I'm wondering why the magic value 3 at all? It seems like this (and the above
> code) should be replaced by a call to checkFunctionOrMethodParameterIndex().
There should be comments explaining the magic number 3.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:829
@@ +828,3 @@
+ if (APType.ugt(3) || APType.slt(0)) {
+ S.Diag(E->getLocStart(), diag::err_attribute_argument_out_of_bounds)
+ << Attr.getName() << 1 << E->getSourceRange();
----------------
That isn't the correct diagnostic to emit. That one is used when a function
attribute that refers to a parameter (like format_arg) uses an index that does
not refer to a parameter. For this one, I would probably modify
err_attribute_argument_outof_range to not be specific to 'init_priority'.
http://reviews.llvm.org/D13263
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
