r199467 - Enhance attribute 'nonnull' to be applicable to parameters directly (infix).
Aaron Ballman
aaron at aaronballman.com
Wed Feb 5 08:40:51 PST 2014
On Fri, Jan 17, 2014 at 1:24 AM, Ted Kremenek <kremenek at apple.com> wrote:
> Author: kremenek
> Date: Fri Jan 17 00:24:56 2014
> New Revision: 199467
>
> URL: http://llvm.org/viewvc/llvm-project?rev=199467&view=rev
> Log:
> Enhance attribute 'nonnull' to be applicable to parameters directly (infix).
>
> This allows the following syntax:
>
> void baz(__attribute__((nonnull)) const char *str);
Is this syntax supported by GCC, or is it a clang extension? I ask
because we currently claim GCC compatibility with this attribute, and
if this is an extension, we should warn as appropriate (and have tests
for the warning).
>
> instead of:
>
> void baz(const char *str) __attribute__((nonnull(1)));
>
> This also extends to Objective-C methods.
>
> The checking logic in Sema is not as clean as I would like. Effectively
> now we need to check both the FunctionDecl/ObjCMethodDecl and the parameters,
> so the point of truth is spread in two places, but the logic isn't that
> cumbersome.
>
> Implements <rdar://problem/14691443>.
>
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/test/Sema/nonnull.c
> cfe/trunk/test/SemaObjC/nonnull.m
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=199467&r1=199466&r2=199467&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Fri Jan 17 00:24:56 2014
> @@ -679,8 +679,9 @@ def NoMips16 : InheritableAttr, TargetSp
>
> def NonNull : InheritableAttr {
> let Spellings = [GNU<"nonnull">, CXX11<"gnu", "nonnull">];
> - let Subjects = SubjectList<[ObjCMethod, FunctionLike, HasFunctionProto],
> - WarnDiag, "ExpectedFunctionOrMethod">;
> + let Subjects = SubjectList<[ObjCMethod, FunctionLike, HasFunctionProto,
> + ParmVar],
> + WarnDiag, "ExpectedFunctionMethodOrParameter">;
> let Args = [VariadicUnsignedArgument<"Args">];
> let AdditionalMembers =
> [{bool isNonNull(unsigned idx) const {
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=199467&r1=199466&r2=199467&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan 17 00:24:56 2014
> @@ -2351,6 +2351,9 @@ def err_attr_wrong_decl : Error<
> def warn_attribute_nonnull_no_pointers : Warning<
> "'nonnull' attribute applied to function with no pointer arguments">,
> InGroup<IgnoredAttributes>;
> +def warn_attribute_nonnull_parm_no_args : Warning<
> + "'nonnull' attribute when used on parameters takes no arguments">,
> + InGroup<IgnoredAttributes>;
I would prefer if this was generalized to take %0 (I can see this
being reused). Also, it reads a bit weird to me. How about:
%0 attribute takes no arguments when used on a parameter
> def warn_attribute_malloc_pointer_only : Warning<
> "'malloc' attribute only applies to functions returning a pointer type">,
> InGroup<IgnoredAttributes>;
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=199467&r1=199466&r2=199467&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Jan 17 00:24:56 2014
> @@ -736,6 +736,7 @@ static void CheckNonNullArguments(Sema &
> const NamedDecl *FDecl,
> const Expr * const *ExprArgs,
> SourceLocation CallSiteLoc) {
> + // Check the attributes attached to the method/function itself.
> for (specific_attr_iterator<NonNullAttr>
> I = FDecl->specific_attr_begin<NonNullAttr>(),
> E = FDecl->specific_attr_end<NonNullAttr>(); I != E; ++I) {
> @@ -747,6 +748,21 @@ static void CheckNonNullArguments(Sema &
> CheckNonNullArgument(S, ExprArgs[*i], CallSiteLoc);
> }
> }
> +
> + // Check the attributes on the parameters.
> + ArrayRef<ParmVarDecl*> parms;
> + if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(FDecl))
> + parms = FD->parameters();
> + else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(FDecl))
> + parms = MD->parameters();
> +
> + unsigned argIndex = 0;
> + for (ArrayRef<ParmVarDecl*>::iterator I = parms.begin(), E = parms.end();
> + I != E; ++I, ++argIndex) {
> + const ParmVarDecl *PVD = *I;
> + if (PVD->hasAttr<NonNullAttr>())
> + CheckNonNullArgument(S, ExprArgs[argIndex], CallSiteLoc);
> + }
> }
>
> /// Handles the checks for format strings, non-POD arguments to vararg
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=199467&r1=199466&r2=199467&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Jan 17 00:24:56 2014
> @@ -1156,6 +1156,36 @@ static void possibleTransparentUnionPoin
> }
> }
>
> +static bool attrNonNullArgCheck(Sema &S, QualType T, const AttributeList &Attr,
> + SourceRange R) {
> + T = T.getNonReferenceType();
> + possibleTransparentUnionPointerType(T);
> +
> + if (!T->isAnyPointerType() && !T->isBlockPointerType()) {
> + S.Diag(Attr.getLoc(), diag::warn_attribute_pointers_only)
> + << Attr.getName() << R;
> + return false;
> + }
> + return true;
> +}
> +
> +static void handleNonNullAttrParameter(Sema &S, ParmVarDecl *D,
> + const AttributeList &Attr) {
> + // Is the argument a pointer type?
> + if (!attrNonNullArgCheck(S, D->getType(), Attr, D->getSourceRange()))
> + return;
> +
> + if (Attr.getNumArgs() > 0) {
> + S.Diag(Attr.getLoc(), diag::warn_attribute_nonnull_parm_no_args)
> + << D->getSourceRange();
> + return;
> + }
> +
> + D->addAttr(::new (S.Context)
> + NonNullAttr(Attr.getRange(), S.Context, 0, 0,
> + Attr.getAttributeSpellingListIndex()));
> +}
> +
> static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) {
> SmallVector<unsigned, 8> NonNullArgs;
> for (unsigned i = 0; i < Attr.getNumArgs(); ++i) {
> @@ -1165,15 +1195,10 @@ static void handleNonNullAttr(Sema &S, D
> return;
>
> // Is the function argument a pointer type?
> - QualType T = getFunctionOrMethodArgType(D, Idx).getNonReferenceType();
> - possibleTransparentUnionPointerType(T);
> -
> - if (!T->isAnyPointerType() && !T->isBlockPointerType()) {
> - // FIXME: Should also highlight argument in decl.
> - S.Diag(Attr.getLoc(), diag::warn_attribute_pointers_only)
> - << Attr.getName() << Ex->getSourceRange();
> + // FIXME: Should also highlight argument in decl in the diagnostic.
> + if (!attrNonNullArgCheck(S, getFunctionOrMethodArgType(D, Idx),
> + Attr, Ex->getSourceRange()))
> continue;
> - }
>
> NonNullArgs.push_back(Idx);
> }
> @@ -3947,7 +3972,12 @@ static void ProcessDeclAttribute(Sema &S
> case AttributeList::AT_Mode: handleModeAttr (S, D, Attr); break;
> case AttributeList::AT_NoCommon:
> handleSimpleAttribute<NoCommonAttr>(S, D, Attr); break;
> - case AttributeList::AT_NonNull: handleNonNullAttr (S, D, Attr); break;
> + case AttributeList::AT_NonNull:
> + if (ParmVarDecl *PVD = dyn_cast<ParmVarDecl>(D))
> + handleNonNullAttrParameter(S, PVD, Attr);
> + else
> + handleNonNullAttr(S, D, Attr);
> + break;
Please hoist this into handleNonNullAttr -- I'm trying to keep logic
out of the switch statement for possible automation purposes.
> case AttributeList::AT_Overloadable:
> handleSimpleAttribute<OverloadableAttr>(S, D, Attr); break;
> case AttributeList::AT_Ownership: handleOwnershipAttr (S, D, Attr); break;
>
> Modified: cfe/trunk/test/Sema/nonnull.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/nonnull.c?rev=199467&r1=199466&r2=199467&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/nonnull.c (original)
> +++ cfe/trunk/test/Sema/nonnull.c Fri Jan 17 00:24:56 2014
> @@ -21,3 +21,14 @@ int main(void) {
>
> void foo(const char *str) __attribute__((nonnull("foo"))); // expected-error{{'nonnull' attribute requires parameter 1 to be an integer constant}}
> void bar(int i) __attribute__((nonnull(1))); // expected-warning {{'nonnull' attribute only applies to pointer arguments}} expected-warning {{'nonnull' attribute applied to function with no pointer arguments}}
> +
> +void baz(__attribute__((nonnull)) const char *str);
> +void baz2(__attribute__((nonnull(1))) const char *str); // expected-warning {{'nonnull' attribute when used on parameters takes no arguments}}
> +void baz3(__attribute__((nonnull)) int x); // expected-warning {{'nonnull' attribute only applies to pointer arguments}}
> +
> +void test_baz() {
> + baz(0); // expected-warning {{null passed to a callee which requires a non-null argument}}
> + baz2(0); // no-warning
> + baz3(0); // no-warning
> +}
> +
>
> Modified: cfe/trunk/test/SemaObjC/nonnull.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/nonnull.m?rev=199467&r1=199466&r2=199467&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/nonnull.m (original)
> +++ cfe/trunk/test/SemaObjC/nonnull.m Fri Jan 17 00:24:56 2014
> @@ -95,3 +95,17 @@ extern void DoSomethingNotNull(void *db)
> }
> @end
>
> +__attribute__((objc_root_class))
> + @interface TestNonNullParameters
> +- (void) doNotPassNullParameterNonPointerArg:(int)__attribute__((nonnull))x; // expected-warning {{'nonnull' attribute only applies to pointer arguments}}
> +- (void) doNotPassNullParameter:(id)__attribute__((nonnull))x;
> +- (void) doNotPassNullParameterArgIndex:(id)__attribute__((nonnull(1)))x; // expected-warning {{'nonnull' attribute when used on parameters takes no arguments}}
> +- (void) doNotPassNullOnMethod:(id)x __attribute__((nonnull(1)));
> + at end
> +
> +void test(TestNonNullParameters *f) {
> + [f doNotPassNullParameter:0]; // expected-warning {{null passed to a callee which requires a non-null argument}}
> + [f doNotPassNullParameterArgIndex:0]; // no-warning
> + [f doNotPassNullOnMethod:0]; // expected-warning {{null passed to a callee which requires a non-null argument}}
> +}
> +
~Aaron
More information about the cfe-commits
mailing list