> On Jun 19, 2015, at 11:10 AM, Aaron Ballman <[email protected]> wrote: > > On Fri, Jun 19, 2015 at 1:51 PM, Douglas Gregor <[email protected] > <mailto:[email protected]>> wrote: >> Author: dgregor >> Date: Fri Jun 19 12:51:05 2015 >> New Revision: 240146 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=240146&view=rev >> Log: >> Introduce type nullability specifiers for C/C++. >> >> Introduces the type specifiers __nonnull, __nullable, and >> __null_unspecified that describe the nullability of the pointer type >> to which the specifier appertains. Nullability type specifiers improve >> on the existing nonnull attributes in a few ways: >> - They apply to types, so one can represent a pointer to a non-null >> pointer, use them in function pointer types, etc. >> - As type specifiers, they are syntactically more lightweight than >> __attribute__s or [[attribute]]s. >> - They can express both the notion of 'should never be null' and >> also 'it makes sense for this to be null', and therefore can more >> easily catch errors of omission where one forgot to annotate the >> nullability of a particular pointer (this will come in a subsequent >> patch). >> >> Nullability type specifiers are maintained as type sugar, and >> therefore have no effect on mangling, encoding, overloading, >> etc. Nonetheless, they will be used for warnings about, e.g., passing >> 'null' to a method that does not accept it. >> >> This is the C/C++ part of rdar://problem/18868820. > > This is great stuff! I have some trivial comments below. > >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=240146&r1=240145&r2=240146&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/Attr.td (original) >> +++ cfe/trunk/include/clang/Basic/Attr.td Fri Jun 19 12:51:05 2015 >> @@ -960,6 +960,22 @@ def ReturnsNonNull : InheritableAttr { >> let Documentation = [Undocumented]; >> } >> >> +// Nullability type attributes. >> +def TypeNonNull : TypeAttr { >> + let Spellings = [Keyword<"__nonnull">]; >> + let Documentation = [Undocumented]; >> +} >> + >> +def TypeNullable : TypeAttr { >> + let Spellings = [Keyword<"__nullable">]; >> + let Documentation = [Undocumented]; >> +} >> + >> +def TypeNullUnspecified : TypeAttr { >> + let Spellings = [Keyword<"__null_unspecified">]; >> + let Documentation = [Undocumented]; >> +} > > Please add documentation for these type attributes (we frown on new > attributes that are flagged as Undocumented).
Good call, r240296. >> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=240146&r1=240145&r2=240146&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Fri Jun 19 >> 12:51:05 2015 >> @@ -65,6 +65,10 @@ def ext_keyword_as_ident : ExtWarn< >> "%select{here|for the remainder of the translation unit}1">, >> InGroup<KeywordCompat>; >> >> +def ext_nullability : Extension< >> + "type nullability specifier %0 is a Clang extension">, >> + InGroup<DiagGroup<"nullability-extension">>; > > Please have %0 quoted in the diagnostic. It’s not necessary; we pass in an IdentifierInfo* here. > >> >> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=240146&r1=240145&r2=240146&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original) >> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Fri Jun 19 12:51:05 2015 >> @@ -687,6 +687,28 @@ void Parser::ParseOpenCLQualifiers(Parse >> AttributeList::AS_Keyword); >> } >> >> +void Parser::ParseNullabilityTypeSpecifiers(ParsedAttributes &attrs) { >> + // Treat these like attributes, even though they're type specifiers. >> + while (true) { >> + switch (Tok.getKind()) { >> + case tok::kw___nonnull: >> + case tok::kw___nullable: >> + case tok::kw___null_unspecified: { >> + IdentifierInfo *AttrName = Tok.getIdentifierInfo(); >> + SourceLocation AttrNameLoc = ConsumeToken(); >> + if (!getLangOpts().ObjC1) >> + Diag(AttrNameLoc, diag::ext_nullability) >> + << AttrName; >> + attrs.addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc, nullptr, 0, >> + AttributeList::AS_Keyword); >> + break; >> + } >> + default: >> + return; > > Would it be more clear to move the default to be the first label in > the switch, and have it break instead of return (in case we would like > to do post-processing of these at some point)? Personally, I find it clearer the way it is. >> >> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=240146&r1=240145&r2=240146&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jun 19 12:51:05 2015 >> @@ -2465,6 +2465,28 @@ static void mergeParamDeclAttributes(Par >> if (!foundAny) newDecl->dropAttrs(); >> } >> >> +static void mergeParamDeclTypes(ParmVarDecl *NewParam, >> + const ParmVarDecl *OldParam, >> + Sema &S) { >> + if (auto Oldnullability = OldParam->getType()->getNullability(S.Context)) >> { >> + if (auto Newnullability = >> NewParam->getType()->getNullability(S.Context)) { > > Can we put the dereference into the assignment expression, since these > are only ever used as dereferenced? We cannot, because we’re getting an Optional<NullabilityKind> here, and we need to check whether we have a value. > >> + if (*Oldnullability != *Newnullability) { >> + S.Diag(NewParam->getLocation(), >> diag::warn_mismatched_nullability_attr) >> + << static_cast<unsigned>(*Newnullability) >> + << static_cast<unsigned>(*Oldnullability); >> + S.Diag(OldParam->getLocation(), diag::note_previous_declaration); >> + } >> + } >> + else { > > Formatting. Okay. >> +/// Distribute a nullability type attribute that cannot be applied to >> +/// the type specifier to a pointer, block pointer, or member pointer >> +/// declarator, complaining if necessary. >> +/// >> +/// \returns true if the nullability annotation was distributed, false >> +/// otherwise. >> +static bool distributeNullabilityTypeAttr(TypeProcessingState &state, >> + QualType type, >> + AttributeList &attr) { >> + Declarator &declarator = state.getDeclarator(); >> + >> + /// Attempt to move the attribute to the specified chunk. >> + auto moveToChunk = [&](DeclaratorChunk &chunk, bool inFunction) -> bool { >> + // If there is already a nullability attribute there, don't add >> + // one. >> + if (hasNullabilityAttr(chunk.getAttrListRef())) >> + return false; >> + >> + // Complain about the nullability qualifier being in the wrong >> + // place. >> + unsigned pointerKind >> + = chunk.Kind == DeclaratorChunk::Pointer ? (inFunction ? 3 : 0) >> + : chunk.Kind == DeclaratorChunk::BlockPointer ? 1 >> + : inFunction? 4 : 2; > > Can we replace the magic numbers with something else? Sure, we can give these meaningful names. - Doug
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
