jdenny created this revision.
jdenny added reviewers: aaron.ballman, hfinkel.
Parameter indices in some attributes (argument_with_type_tag,
pointer_with_type_tag, nonnull, ownership_takes, ownership_holds, and
ownership_returns) are specified in source as one-origin including any
this parameter, are stored as zero-origin excluding any this
parameter, and were erroneously printing (-ast-print) as the stored
values. This patch fixes those cases by printing re-incremented
values.
An alternative solution is to store only the original values, but that
requires modifying all users. I tried that for nonnull and found that
it required repeating index adjustment logic in many places while this
patch encapsulates that logic in relatively few places.
Another alternative is to store both sets of values, but that would be
less efficient.
This patch also fixes argument_with_type_tag and pointer_with_type_tag
not to print their fake IsPointer arguments.
https://reviews.llvm.org/D43248
Files:
include/clang/Basic/Attr.td
lib/Sema/SemaDeclAttr.cpp
test/Sema/attr-print.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -738,6 +738,42 @@
}
};
+ class VariadicParamIndexArgument : public VariadicArgument {
+ std::string DisallowImplicitThisParamName;
+
+ protected:
+ // Assumed to receive a parameter: raw_ostream OS.
+ virtual void writeValueImpl(raw_ostream &OS) const {
+ OS << " OS << (Val + 1";
+ if (!DisallowImplicitThisParamName.empty())
+ OS << " + get" << DisallowImplicitThisParamName << "()";
+ OS << ");\n";
+ }
+ public:
+ VariadicParamIndexArgument(const Record &Arg, StringRef Attr)
+ : VariadicArgument(Arg, Attr, "unsigned"),
+ DisallowImplicitThisParamName(
+ Arg.getValueAsString("DisallowImplicitThisParamName")) {}
+ };
+
+ class ParamIndexArgument : public SimpleArgument {
+ std::string DisallowImplicitThisParamName;
+
+ protected:
+ void writeValue(raw_ostream &OS) const override {
+ OS << "\" << (get" << getUpperName() << "() + 1";
+ if (!DisallowImplicitThisParamName.empty())
+ OS << " + get" << DisallowImplicitThisParamName << "()";
+ OS << ") << \"";
+ }
+
+ public:
+ ParamIndexArgument(const Record &Arg, StringRef Attr)
+ : SimpleArgument(Arg, Attr, "unsigned"),
+ DisallowImplicitThisParamName(
+ Arg.getValueAsString("DisallowImplicitThisParamName")) {}
+ };
+
// Unique the enums, but maintain the original declaration ordering.
std::vector<StringRef>
uniqueEnumsInOrder(const std::vector<StringRef> &enums) {
@@ -1237,6 +1273,10 @@
Ptr = llvm::make_unique<VariadicEnumArgument>(Arg, Attr);
else if (ArgName == "VariadicExprArgument")
Ptr = llvm::make_unique<VariadicExprArgument>(Arg, Attr);
+ else if (ArgName == "VariadicParamIndexArgument")
+ Ptr = llvm::make_unique<VariadicParamIndexArgument>(Arg, Attr);
+ else if (ArgName == "ParamIndexArgument")
+ Ptr = llvm::make_unique<ParamIndexArgument>(Arg, Attr);
else if (ArgName == "VersionArgument")
Ptr = llvm::make_unique<VersionArgument>(Arg, Attr);
Index: test/Sema/attr-print.cpp
===================================================================
--- /dev/null
+++ test/Sema/attr-print.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 %s -ast-print | FileCheck %s
+
+void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
+// CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
+void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 1, 2)));
+// CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 1, 2)));
+
+void nn(int *, int *) __attribute__((nonnull(1, 2)));
+// CHECK: void nn(int *, int *) __attribute__((nonnull(1, 2)));
+
+void ownt(int *, int *) __attribute__((ownership_takes(foo, 1, 2)));
+// CHECK: void ownt(int *, int *) __attribute__((ownership_takes(foo, 1, 2)));
+void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
+// CHECK: void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
+void ownr(int) __attribute__((ownership_returns(foo, 1)));
+// CHECK: void ownr(int) __attribute__((ownership_returns(foo, 1)));
+
+class C {
+ void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
+ // CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
+ void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
+ // CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
+
+ void nn(int *, int *) __attribute__((nonnull(2, 3)));
+ // CHECK: void nn(int *, int *) __attribute__((nonnull(2, 3)));
+
+ void ownt(int *, int *) __attribute__((ownership_takes(foo, 2, 3)));
+ // CHECK: void ownt(int *, int *) __attribute__((ownership_takes(foo, 2, 3)));
+ void ownh(int *, int *) __attribute__((ownership_holds(foo, 2, 3)));
+ // CHECK: void ownh(int *, int *) __attribute__((ownership_holds(foo, 2, 3)));
+ void ownr(int) __attribute__((ownership_returns(foo, 2)));
+ // CHECK: void ownr(int) __attribute__((ownership_returns(foo, 2)));
+};
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -314,7 +314,8 @@
template <typename AttrInfo>
static bool checkFunctionOrMethodParameterIndex(
Sema &S, const Decl *D, const AttrInfo &Attr, unsigned AttrArgNum,
- const Expr *IdxExpr, uint64_t &Idx, bool AllowImplicitThis = false) {
+ const Expr *IdxExpr, uint64_t &Idx, bool AllowImplicitThis = false,
+ bool *DisallowImplicitThisParam = nullptr) {
assert(isFunctionOrMethodOrBlock(D));
// In C++ the implicit 'this' function parameter also counts.
@@ -349,7 +350,11 @@
return false;
}
--Idx;
+ if (DisallowImplicitThisParam)
+ *DisallowImplicitThisParam = true;
}
+ else if (DisallowImplicitThisParam)
+ *DisallowImplicitThisParam = false;
return true;
}
@@ -1452,11 +1457,15 @@
}
static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) {
+ // Init of DisallowImplicitThisParam is for case of no values, but no values
+ // means it doesn't matter.
+ bool DisallowImplicitThisParam = false;
SmallVector<unsigned, 8> NonNullArgs;
for (unsigned I = 0; I < Attr.getNumArgs(); ++I) {
Expr *Ex = Attr.getArgAsExpr(I);
uint64_t Idx;
- if (!checkFunctionOrMethodParameterIndex(S, D, Attr, I + 1, Ex, Idx))
+ if (!checkFunctionOrMethodParameterIndex(S, D, Attr, I + 1, Ex, Idx, false,
+ &DisallowImplicitThisParam))
return;
// Is the function argument a pointer type?
@@ -1492,6 +1501,7 @@
llvm::array_pod_sort(Start, Start + Size);
D->addAttr(::new (S.Context)
NonNullAttr(Attr.getRange(), S.Context, Start, Size,
+ DisallowImplicitThisParam,
Attr.getAttributeSpellingListIndex()));
}
@@ -1706,11 +1716,15 @@
Module = &S.PP.getIdentifierTable().get(ModuleName);
}
+ // Init of DisallowImplicitThisParam is for case of no values, but no values
+ // means it doesn't matter.
+ bool DisallowImplicitThisParam = false;
SmallVector<unsigned, 8> OwnershipArgs;
for (unsigned i = 1; i < AL.getNumArgs(); ++i) {
Expr *Ex = AL.getArgAsExpr(i);
uint64_t Idx;
- if (!checkFunctionOrMethodParameterIndex(S, D, AL, i, Ex, Idx))
+ if (!checkFunctionOrMethodParameterIndex(S, D, AL, i, Ex, Idx, false,
+ &DisallowImplicitThisParam))
return;
// Is the function argument a pointer type?
@@ -1766,6 +1780,7 @@
D->addAttr(::new (S.Context)
OwnershipAttr(AL.getLoc(), S.Context, Module, start, size,
+ DisallowImplicitThisParam,
AL.getAttributeSpellingListIndex()));
}
@@ -4583,14 +4598,18 @@
return;
}
+ bool DisallowImplicitThisParam;
+
uint64_t ArgumentIdx;
if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 2, Attr.getArgAsExpr(1),
- ArgumentIdx))
+ ArgumentIdx, false,
+ &DisallowImplicitThisParam))
return;
uint64_t TypeTagIdx;
if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 3, Attr.getArgAsExpr(2),
- TypeTagIdx))
+ TypeTagIdx, false,
+ &DisallowImplicitThisParam))
return;
bool IsPointer = (Attr.getName()->getName() == "pointer_with_type_tag");
@@ -4605,7 +4624,8 @@
D->addAttr(::new (S.Context)
ArgumentWithTypeTagAttr(Attr.getRange(), S.Context, ArgumentKind,
- ArgumentIdx, TypeTagIdx, IsPointer,
+ ArgumentIdx, TypeTagIdx,
+ DisallowImplicitThisParam, IsPointer,
Attr.getAttributeSpellingListIndex()));
}
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -163,6 +163,32 @@
class VariadicExprArgument<string name> : Argument<name, 1>;
class VariadicStringArgument<string name> : Argument<name, 1>;
+// Like VariadicUnsignedArgument except values represent function parameter
+// indices. That is, they are stored with zero-origin indexing, but they
+// print with one-origin indexing as in the original source code. Moreover, if
+// there's an implicit this parameter whose index is not allowed as a value,
+// then the stored values are decremented by one more relative to the printed
+// original values. The burden for storing properly decremented values is on
+// the user (see checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp), but
+// this class handles printing the re-incremented values.
+class VariadicParamIndexArgument<string name,
+ string disallowImplicitThisParamName>
+ : VariadicUnsignedArgument<name> {
+ // In the enclosing attribute, this is the name of the fake BoolArgument
+ // that records whether both (1) the attribute is on a function with an
+ // implicit this parameter and (2) the this parameter's index is not allowed
+ // as a value here. Empty string if there's no such BoolArgument because
+ // it would always be false.
+ string DisallowImplicitThisParamName = disallowImplicitThisParamName;
+}
+
+// Like VariadicParamIndexArgument but for a single non-optional function
+// parameter index.
+class ParamIndexArgument<string name, string disallowImplicitThisParamName>
+ : UnsignedArgument<name> {
+ string DisallowImplicitThisParamName = disallowImplicitThisParamName;
+}
+
// A version of the form major.minor[.subminor].
class VersionArgument<string name, bit opt = 0> : Argument<name, opt>;
@@ -1376,7 +1402,8 @@
let Spellings = [GCC<"nonnull">];
let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], WarnDiag,
"functions, methods, and parameters">;
- let Args = [VariadicUnsignedArgument<"Args">];
+ let Args = [VariadicParamIndexArgument<"Args", "DisallowImplicitThisParam">,
+ BoolArgument<"DisallowImplicitThisParam", 0, /*fake*/ 1>];
let AdditionalMembers =
[{bool isNonNull(unsigned idx) const {
if (!args_size())
@@ -1654,7 +1681,9 @@
Returns;
}
}];
- let Args = [IdentifierArgument<"Module">, VariadicUnsignedArgument<"Args">];
+ let Args = [IdentifierArgument<"Module">,
+ VariadicParamIndexArgument<"Args", "DisallowImplicitThisParam">,
+ BoolArgument<"DisallowImplicitThisParam", 0, /*fake*/ 1>];
let Subjects = SubjectList<[HasFunctionProto]>;
let Documentation = [Undocumented];
}
@@ -2449,9 +2478,10 @@
let Spellings = [GNU<"argument_with_type_tag">,
GNU<"pointer_with_type_tag">];
let Args = [IdentifierArgument<"ArgumentKind">,
- UnsignedArgument<"ArgumentIdx">,
- UnsignedArgument<"TypeTagIdx">,
- BoolArgument<"IsPointer">];
+ ParamIndexArgument<"ArgumentIdx", "DisallowImplicitThisParam">,
+ ParamIndexArgument<"TypeTagIdx", "DisallowImplicitThisParam">,
+ BoolArgument<"DisallowImplicitThisParam", 0, /*fake*/ 1>,
+ BoolArgument<"IsPointer", 0, /*fake*/ 1>];
let HasCustomParsing = 1;
let Documentation = [ArgumentWithTypeTagDocs, PointerWithTypeTagDocs];
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
