From: Jordan Rose Date: Thu, 10 Nov 2016 23:28:17 +0000 (+0000) Subject: Accept nullability qualifiers on array parameters. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b0eb2871c6471b7f99a3d8a25300fb57bb266804;p=clang Accept nullability qualifiers on array parameters. Since array parameters decay to pointers, '_Nullable' and friends should be available for use there as well. This is especially important for parameters that are typedefs of arrays. The unsugared syntax for this follows the syntax for 'static'-sized arrays in C: void test(int values[_Nullable]); This syntax was previously accepted but the '_Nullable' (and any other attributes) were silently discarded. However, applying '_Nullable' to a typedef was previously rejected and is now accepted; therefore, it may be necessary to test for the presence of this feature: #if __has_feature(nullability_on_arrays) One important change here is that DecayedTypes don't always immediately contain PointerTypes anymore; they may contain an AttributedType instead. This only affected one place in-tree, so I would guess it's not likely to cause problems elsewhere. This commit does not change -Wnullability-completeness just yet. I want to think about whether it's worth doing something special to avoid breaking existing clients that compile with -Werror. It also doesn't change '#pragma clang assume_nonnull' behavior, which currently treats the following two declarations as equivalent: #pragma clang assume_nonnull begin void test(void *pointers[]); #pragma clang assume_nonnull end void test(void * _Nonnull pointers[]); This is not the desired behavior, but changing it would break backwards-compatibility. Most likely the best answer is going to be adding a new warning. Part of rdar://problem/25846421 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@286519 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h index 1208cb7311..a33799837a 100644 --- a/include/clang/AST/Type.h +++ b/include/clang/AST/Type.h @@ -2266,19 +2266,15 @@ public: /// Represents a pointer type decayed from an array or function type. class DecayedType : public AdjustedType { - DecayedType(QualType OriginalType, QualType DecayedPtr, QualType CanonicalPtr) - : AdjustedType(Decayed, OriginalType, DecayedPtr, CanonicalPtr) { - assert(isa(getAdjustedType())); - } + inline + DecayedType(QualType OriginalType, QualType Decayed, QualType Canonical); friend class ASTContext; // ASTContext creates these. public: QualType getDecayedType() const { return getAdjustedType(); } - QualType getPointeeType() const { - return cast(getDecayedType())->getPointeeType(); - } + inline QualType getPointeeType() const; static bool classof(const Type *T) { return T->getTypeClass() == Decayed; } }; @@ -5962,6 +5958,23 @@ inline const ArrayType *Type::castAsArrayTypeUnsafe() const { return cast(getUnqualifiedDesugaredType()); } +DecayedType::DecayedType(QualType OriginalType, QualType DecayedPtr, + QualType CanonicalPtr) + : AdjustedType(Decayed, OriginalType, DecayedPtr, CanonicalPtr) { +#ifndef NDEBUG + QualType Adjusted = getAdjustedType(); + (void)AttributedType::stripOuterNullability(Adjusted); + assert(isa(Adjusted)); +#endif +} + +QualType DecayedType::getPointeeType() const { + QualType Decayed = getDecayedType(); + (void)AttributedType::stripOuterNullability(Decayed); + return cast(Decayed)->getPointeeType(); +} + + } // end namespace clang #endif diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 2f4befc8b7..23ec47d02f 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -3122,10 +3122,14 @@ public: /// method) or an Objective-C property attribute, rather than as an /// underscored type specifier. /// + /// \param allowArrayTypes Whether to accept nullability specifiers on an + /// array type (e.g., because it will decay to a pointer). + /// /// \returns true if nullability cannot be applied, false otherwise. bool checkNullabilityTypeSpecifier(QualType &type, NullabilityKind nullability, SourceLocation nullabilityLoc, - bool isContextSensitive); + bool isContextSensitive, + bool allowArrayTypes); /// \brief Stmt attributes - this routine is the top level dispatcher. StmtResult ProcessStmtAttributes(Stmt *Stmt, AttributeList *Attrs, diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index d2c35851ee..c48ab9a44a 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -4913,7 +4913,15 @@ QualType ASTContext::getArrayDecayedType(QualType Ty) const { QualType PtrTy = getPointerType(PrettyArrayType->getElementType()); // int x[restrict 4] -> int *restrict - return getQualifiedType(PtrTy, PrettyArrayType->getIndexTypeQualifiers()); + QualType Result = getQualifiedType(PtrTy, + PrettyArrayType->getIndexTypeQualifiers()); + + // int x[_Nullable] -> int * _Nullable + if (auto Nullability = Ty->getNullability(*this)) { + Result = const_cast(this)->getAttributedType( + AttributedType::getNullabilityAttrKind(*Nullability), Result, Result); + } + return Result; } QualType ASTContext::getBaseElementType(const ArrayType *array) const { diff --git a/lib/CodeGen/CGDebugInfo.cpp b/lib/CodeGen/CGDebugInfo.cpp index f2f1918c90..ec4c6e3584 100644 --- a/lib/CodeGen/CGDebugInfo.cpp +++ b/lib/CodeGen/CGDebugInfo.cpp @@ -2416,12 +2416,18 @@ static QualType UnwrapTypeForDebugInfo(QualType T, const ASTContext &C) { case Type::SubstTemplateTypeParm: T = cast(T)->getReplacementType(); break; - case Type::Auto: + case Type::Auto: { QualType DT = cast(T)->getDeducedType(); assert(!DT.isNull() && "Undeduced types shouldn't reach here."); T = DT; break; } + case Type::Adjusted: + case Type::Decayed: + // Decayed and adjusted types use the adjusted type in LLVM and DWARF. + T = cast(T)->getAdjustedType(); + break; + } assert(T != LastT && "Type unwrapping failed to unwrap!"); (void)LastT; @@ -2540,11 +2546,6 @@ llvm::DIType *CGDebugInfo::CreateTypeNode(QualType Ty, llvm::DIFile *Unit) { return CreateType(cast(Ty)); case Type::Pointer: return CreateType(cast(Ty), Unit); - case Type::Adjusted: - case Type::Decayed: - // Decayed and adjusted types use the adjusted type in LLVM and DWARF. - return CreateType( - cast(cast(Ty)->getAdjustedType()), Unit); case Type::BlockPointer: return CreateType(cast(Ty), Unit); case Type::Typedef: @@ -2580,6 +2581,8 @@ llvm::DIType *CGDebugInfo::CreateTypeNode(QualType Ty, llvm::DIFile *Unit) { case Type::Auto: case Type::Attributed: + case Type::Adjusted: + case Type::Decayed: case Type::Elaborated: case Type::Paren: case Type::SubstTemplateTypeParm: diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp index a9e7dcbe54..f3a57e3348 100644 --- a/lib/Lex/PPMacroExpansion.cpp +++ b/lib/Lex/PPMacroExpansion.cpp @@ -1108,6 +1108,7 @@ static bool HasFeature(const Preprocessor &PP, StringRef Feature) { .Case("cxx_rtti", LangOpts.RTTI && LangOpts.RTTIData) .Case("enumerator_attributes", true) .Case("nullability", true) + .Case("nullability_on_arrays", true) .Case("memory_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Memory)) .Case("thread_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Thread)) .Case("dataflow_sanitizer", LangOpts.Sanitize.has(SanitizerKind::DataFlow)) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index ea942eb69a..d155432cbd 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -6250,8 +6250,7 @@ void Parser::ParseBracketDeclarator(Declarator &D) { T.consumeClose(); - ParsedAttributes attrs(AttrFactory); - MaybeParseCXX11Attributes(attrs); + MaybeParseCXX11Attributes(DS.getAttributes()); // Remember that we parsed a array type, and remember its features. D.AddTypeInfo(DeclaratorChunk::getArray(DS.getTypeQualifiers(), @@ -6259,7 +6258,7 @@ void Parser::ParseBracketDeclarator(Declarator &D) { NumElements.get(), T.getOpenLocation(), T.getCloseLocation()), - attrs, T.getCloseLocation()); + DS.getAttributes(), T.getCloseLocation()); } /// Diagnose brackets before an identifier. diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index c8f4b68dd7..fcafd7148e 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -3484,6 +3484,39 @@ static void checkNullabilityConsistency(TypeProcessingState &state, << static_cast(pointerKind); } +/// Returns true if any of the declarator chunks before \p endIndex include a +/// level of indirection: array, pointer, reference, or pointer-to-member. +/// +/// Because declarator chunks are stored in outer-to-inner order, testing +/// every chunk before \p endIndex is testing all chunks that embed the current +/// chunk as part of their type. +/// +/// It is legal to pass the result of Declarator::getNumTypeObjects() as the +/// end index, in which case all chunks are tested. +static bool hasOuterPointerLikeChunk(const Declarator &D, unsigned endIndex) { + unsigned i = endIndex; + while (i != 0) { + // Walk outwards along the declarator chunks. + --i; + const DeclaratorChunk &DC = D.getTypeObject(i); + switch (DC.Kind) { + case DeclaratorChunk::Paren: + break; + case DeclaratorChunk::Array: + case DeclaratorChunk::Pointer: + case DeclaratorChunk::Reference: + case DeclaratorChunk::MemberPointer: + return true; + case DeclaratorChunk::Function: + case DeclaratorChunk::BlockPointer: + case DeclaratorChunk::Pipe: + // These are invalid anyway, so just ignore. + break; + } + } + return false; +} + static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, QualType declSpecType, TypeSourceInfo *TInfo) { @@ -3935,31 +3968,13 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, // C99 6.7.5.2p1: ... and then only in the outermost array type // derivation. - unsigned x = chunkIndex; - while (x != 0) { - // Walk outwards along the declarator chunks. - x--; - const DeclaratorChunk &DC = D.getTypeObject(x); - switch (DC.Kind) { - case DeclaratorChunk::Paren: - continue; - case DeclaratorChunk::Array: - case DeclaratorChunk::Pointer: - case DeclaratorChunk::Reference: - case DeclaratorChunk::MemberPointer: - S.Diag(DeclType.Loc, diag::err_array_static_not_outermost) << - (ASM == ArrayType::Static ? "'static'" : "type qualifier"); - if (ASM == ArrayType::Static) - ASM = ArrayType::Normal; - ATI.TypeQuals = 0; - D.setInvalidType(true); - break; - case DeclaratorChunk::Function: - case DeclaratorChunk::BlockPointer: - case DeclaratorChunk::Pipe: - // These are invalid anyway, so just ignore. - break; - } + if (hasOuterPointerLikeChunk(D, chunkIndex)) { + S.Diag(DeclType.Loc, diag::err_array_static_not_outermost) << + (ASM == ArrayType::Static ? "'static'" : "type qualifier"); + if (ASM == ArrayType::Static) + ASM = ArrayType::Normal; + ATI.TypeQuals = 0; + D.setInvalidType(true); } } const AutoType *AT = T->getContainedAutoType(); @@ -5834,7 +5849,8 @@ static bool handleMSPointerTypeQualifierAttr(TypeProcessingState &State, bool Sema::checkNullabilityTypeSpecifier(QualType &type, NullabilityKind nullability, SourceLocation nullabilityLoc, - bool isContextSensitive) { + bool isContextSensitive, + bool allowOnArrayType) { // We saw a nullability type specifier. If this is the first one for // this file, note that. FileID file = getNullabilityCompletenessCheckFileID(*this, nullabilityLoc); @@ -5904,7 +5920,8 @@ bool Sema::checkNullabilityTypeSpecifier(QualType &type, } // If this definitely isn't a pointer type, reject the specifier. - if (!desugared->canHaveNullability()) { + if (!desugared->canHaveNullability() && + !(allowOnArrayType && desugared->isArrayType())) { Diag(nullabilityLoc, diag::err_nullability_nonpointer) << DiagNullabilityKind(nullability, isContextSensitive) << type; return true; @@ -5913,7 +5930,13 @@ bool Sema::checkNullabilityTypeSpecifier(QualType &type, // For the context-sensitive keywords/Objective-C property // attributes, require that the type be a single-level pointer. if (isContextSensitive) { - QualType pointeeType = desugared->getPointeeType(); + // Make sure that the pointee isn't itself a pointer type. + const Type *pointeeType; + if (desugared->isArrayType()) + pointeeType = desugared->getArrayElementTypeNoTypeQual(); + else + pointeeType = desugared->getPointeeType().getTypePtr(); + if (pointeeType->isAnyPointerType() || pointeeType->isObjCObjectPointerType() || pointeeType->isMemberPointerType()) { @@ -6668,12 +6691,22 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type, // don't want to distribute the nullability specifier past any // dependent type, because that complicates the user model. if (type->canHaveNullability() || type->isDependentType() || + type->isArrayType() || !distributeNullabilityTypeAttr(state, type, attr)) { + unsigned endIndex; + if (TAL == TAL_DeclChunk) + endIndex = state.getCurrentChunkIndex(); + else + endIndex = state.getDeclarator().getNumTypeObjects(); + bool allowOnArrayType = + state.getDeclarator().isPrototypeContext() && + !hasOuterPointerLikeChunk(state.getDeclarator(), endIndex); if (state.getSema().checkNullabilityTypeSpecifier( type, mapNullabilityAttrKind(attr.getKind()), attr.getLoc(), - attr.isContextSensitiveKeywordAttribute())) { + attr.isContextSensitiveKeywordAttribute(), + allowOnArrayType)) { attr.setInvalid(); } diff --git a/test/Parser/nullability.c b/test/Parser/nullability.c index 7117bce4bb..d70087df9f 100644 --- a/test/Parser/nullability.c +++ b/test/Parser/nullability.c @@ -11,6 +11,14 @@ _Nonnull int *ptr2; // no-warning # error Nullability should always be supported #endif +#if !__has_feature(nullability_on_arrays) +# error Nullability on array parameters should always be supported +#endif + #if !__has_extension(nullability) # error Nullability should always be supported as an extension #endif + +#if !__has_extension(nullability_on_arrays) +# error Nullability on array parameters should always be supported as an extension +#endif diff --git a/test/Sema/nullability.c b/test/Sema/nullability.c index 9d3145d068..a0247e5af8 100644 --- a/test/Sema/nullability.c +++ b/test/Sema/nullability.c @@ -195,3 +195,55 @@ void binary_conditional_expr() { p = noneP ?: unspecifiedP; p = noneP ?: noneP; } + +extern int GLOBAL_LENGTH; + +// Nullability can appear on arrays when the arrays are in parameter lists. +void arrays(int ints[_Nonnull], + void *ptrs[_Nullable], + void **nestedPtrs[_Nullable], + void * _Null_unspecified * _Nonnull nestedPtrs2[_Nullable], + int fixedSize[_Nonnull 2], + int staticSize[_Nonnull static 2], + int staticSize2[static _Nonnull 2], + int starSize[_Nonnull *], + int vla[_Nonnull GLOBAL_LENGTH], + void ** _Nullable reference); +void testDecayedType() { + int produceAnErrorMessage = arrays; // expected-warning {{incompatible pointer to integer conversion initializing 'int' with an expression of type 'void (int * _Nonnull, void ** _Nullable, void *** _Nullable, void * _Null_unspecified * _Nonnull * _Nullable, int * _Nonnull, int * _Nonnull, int * _Nonnull, int * _Nonnull, int * _Nonnull, void ** _Nullable)'}} +} + +int notInFunction[_Nullable 3]; // expected-error {{nullability specifier '_Nullable' cannot be applied to non-pointer type 'int [3]'}} + +void nestedArrays(int x[5][_Nonnull 1]) {} // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int [1]'}} +void nestedArrays2(int x[5][_Nonnull 1][2]) {} // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int [1][2]'}} +void nestedArraysOK(int x[_Nonnull 5][1]) {} // ok + +void nullabilityOnBase(_Nonnull int x[1], // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int'}} + int _Nonnull y[1]); // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int'}} + +typedef int INTS[4]; +typedef int BAD_INTS[_Nonnull 4]; // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int [4]'}} + +void typedefTest(INTS _Nonnull x, + _Nonnull INTS xx, + INTS _Nonnull y[2], // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'INTS' (aka 'int [4]')}} + INTS z[_Nonnull 2]); + +INTS _Nonnull x; // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'INTS' (aka 'int [4]')}} +_Nonnull INTS x; // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'INTS' (aka 'int [4]')}} + +void arraysInBlocks() { + typedef int INTS[4]; + void (^simple)(int [_Nonnull 2]) = ^(int x[_Nonnull 2]) {}; + simple(0); // expected-warning {{null passed to a callee that requires a non-null argument}} + void (^nested)(void *_Nullable x[_Nonnull 2]) = ^(void *_Nullable x[_Nonnull 2]) {}; + nested(0); // expected-warning {{null passed to a callee that requires a non-null argument}} + void (^nestedBad)(int x[2][_Nonnull 2]) = // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int [2]'}} + ^(int x[2][_Nonnull 2]) {}; // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int [2]'}} + + void (^withTypedef)(INTS _Nonnull) = ^(INTS _Nonnull x) {}; + withTypedef(0); // expected-warning {{null passed to a callee that requires a non-null argument}} + void (^withTypedefBad)(INTS _Nonnull [2]) = // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'INTS' (aka 'int [4]')}} + ^(INTS _Nonnull x[2]) {}; // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'INTS' (aka 'int [4]')}} +} diff --git a/test/SemaCXX/nullability.cpp b/test/SemaCXX/nullability.cpp index 2af25730b6..160741b140 100644 --- a/test/SemaCXX/nullability.cpp +++ b/test/SemaCXX/nullability.cpp @@ -117,3 +117,16 @@ void ConditionalExpr(bool c) { p = c ? nullableD : nonnullB; // expected-warning{{implicit conversion from nullable pointer 'Base * _Nullable' to non-nullable pointer type 'Base * _Nonnull}} p = c ? nullableD : nullableB; // expected-warning{{implicit conversion from nullable pointer 'Base * _Nullable' to non-nullable pointer type 'Base * _Nonnull}} } + +void arraysInLambdas() { + typedef int INTS[4]; + auto simple = [](int [_Nonnull 2]) {}; + simple(nullptr); // expected-warning {{null passed to a callee that requires a non-null argument}} + auto nested = [](void *_Nullable [_Nonnull 2]) {}; + nested(nullptr); // expected-warning {{null passed to a callee that requires a non-null argument}} + auto nestedBad = [](int [2][_Nonnull 2]) {}; // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int [2]'}} + + auto withTypedef = [](INTS _Nonnull) {}; + withTypedef(nullptr); // expected-warning {{null passed to a callee that requires a non-null argument}} + auto withTypedefBad = [](INTS _Nonnull[2]) {}; // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'INTS' (aka 'int [4]')}} +} diff --git a/test/SemaObjC/nullability.m b/test/SemaObjC/nullability.m index fbf014c0f9..cbfe132d14 100644 --- a/test/SemaObjC/nullability.m +++ b/test/SemaObjC/nullability.m @@ -256,3 +256,26 @@ void conditional_expr(int c) { p = c ? noneP : unspecifiedP; p = c ? noneP : noneP; } + +typedef int INTS[4]; +@interface ArraysInMethods +- (void)simple:(int [_Nonnull 2])x; +- (void)nested:(void *_Nullable [_Nonnull 2])x; +- (void)nestedBad:(int [2][_Nonnull 2])x; // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int [2]'}} + +- (void)withTypedef:(INTS _Nonnull)x; +- (void)withTypedefBad:(INTS _Nonnull[2])x; // expected-error{{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'INTS' (aka 'int [4]')}} + +- (void)simpleSugar:(nonnull int [2])x; +- (void)nestedSugar:(nonnull void *_Nullable [2])x; // expected-error {{nullability keyword 'nonnull' cannot be applied to multi-level pointer type 'void * _Nullable [2]'}} expected-note {{use nullability type specifier '_Nonnull' to affect the innermost pointer type of 'void * _Nullable [2]'}} +- (void)sugarWithTypedef:(nonnull INTS)x; +@end + +void test(ArraysInMethods *obj) { + [obj simple:0]; // expected-warning {{null passed to a callee that requires a non-null argument}} + [obj nested:0]; // expected-warning {{null passed to a callee that requires a non-null argument}} + [obj withTypedef:0]; // expected-warning {{null passed to a callee that requires a non-null argument}} + + [obj simpleSugar:0]; // expected-warning {{null passed to a callee that requires a non-null argument}} + [obj sugarWithTypedef:0]; // expected-warning {{null passed to a callee that requires a non-null argument}} +}