r240158 - Check for consistent use of nullability type specifiers in a header.

[Douglas Gregor]

> On Jun 20, 2015, at 12:48 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> 
> On Fri, Jun 19, 2015 at 2:27 PM, Douglas Gregor <dgregor at apple.com <mailto:dgregor at apple.com>> wrote:
>> Author: dgregor
>> Date: Fri Jun 19 13:27:45 2015
>> New Revision: 240158
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=240158&view=rev
>> Log:
>> Check for consistent use of nullability type specifiers in a header.
>> 
>> Adds a new warning (under -Wnullability-completeness) that complains
>> about pointer, block pointer, or member pointer declarations that have
>> not been annotated with nullability information (directly or inferred)
>> within a header that contains some nullability annotations. This is
>> intended to be used to help maintain the completeness of nullability
>> information within a header that has already been audited.
>> 
>> Note that, for performance reasons, this warning will underrepresent
>> the number of non-annotated pointers in the case where more than one
>> pointer is seen before the first nullability type specifier, because
>> we're only tracking one piece of information per header. Part of
>> rdar://problem/18868820.
>> 
>> Added:
>>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-1.h   (with props)
>>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-2.h   (with props)
>>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-3.h   (with props)
>>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-4.h   (with props)
>>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-5.h   (with props)
>>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-6.h   (with props)
>>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-7.h   (with props)
>>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-8.h   (with props)
>>    cfe/trunk/test/SemaObjCXX/nullability-consistency.mm
>> Modified:
>>    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>    cfe/trunk/include/clang/Sema/AttributeList.h
>>    cfe/trunk/include/clang/Sema/Sema.h
>>    cfe/trunk/lib/Parse/ParseObjc.cpp
>>    cfe/trunk/lib/Sema/SemaType.cpp
>>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-pragmas-1.h
>>    cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
>> 
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=240158&r1=240157&r2=240158&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Jun 19 13:27:45 2015
>> @@ -251,6 +251,7 @@ def NewlineEOF : DiagGroup<"newline-eof"
>> def Nullability : DiagGroup<"nullability">;
>> def NullabilityDeclSpec : DiagGroup<"nullability-declspec">;
>> def NullableToNonNullConversion : DiagGroup<"nullable-to-nonnull-conversion">;
>> +def NullabilityCompleteness : DiagGroup<"nullability-completeness">;
>> def NullArithmetic : DiagGroup<"null-arithmetic">;
>> def NullCharacter : DiagGroup<"null-character">;
>> def NullDereference : DiagGroup<"null-dereference">;
>> 
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=240158&r1=240157&r2=240158&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jun 19 13:27:45 2015
>> @@ -7718,6 +7718,11 @@ def warn_null_resettable_setter : Warnin
>>   "synthesized setter %0 for null_resettable property %1 does not handle nil">,
>>   InGroup<Nullability>;
>> 
>> +def warn_nullability_missing : Warning<
>> +  "%select{pointer|block pointer|member pointer}0 is missing a nullability "
>> +  "type specifier (__nonnull, __nullable, or __null_unspecified)">,
>> +  InGroup<NullabilityCompleteness>;
>> +
>> }
>> 
>> } // end of sema component.
>> 
>> Modified: cfe/trunk/include/clang/Sema/AttributeList.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=240158&r1=240157&r2=240158&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Sema/AttributeList.h (original)
>> +++ cfe/trunk/include/clang/Sema/AttributeList.h Fri Jun 19 13:27:45 2015
>> @@ -81,6 +81,8 @@ public:
>>     AS_Declspec,
>>     /// __ptr16, alignas(...), etc.
>>     AS_Keyword,
>> +    /// Context-sensitive version of a keyword attribute.
>> +    AS_ContextSensitiveKeyword,
> 
> I don't like this approach. These are meant to mirror spellings
> generated by tablegen that the user can write, so this breaks the
> attribute syntax model. Put slightly differently, as far as the table
> definition for an attribute is concerned, *all* keywords are currently
> context sensitive in that we don't (and likely will never) table
> generate parsing for attribute keywords. So this provides no useful
> distinction for the table definition of keyword attributes.

The tablgen-generated table we’re talking about generates part of the documentation; it’s not some deep parser generation logic where the lack of information about the potential contexts for the keyword is signifiant. We generally treat keywords and context-sensitive keywords the same way throughout, so using the syntax kind to describe this syntactic distinction seems totally reasonable to me.

> 
> From what I can tell, context sensitivity is only used for error
> reporting purposes (whether we want to print __nonnull or nonnull in a
> diagnostic). That's part of the spelling of the attribute itself,
> which AttributeList already tracks. Can that be used instead? If not,
> the previous approach was marginally less heinous (though still
> problematic).


To move this information into the spelling of the attribute itself, we’ll need to either push the hack somewhere else (AttributeList::getKind) or admit context-sensitivity further into TableGen with a ContextSensitiveKeyword<“nonnull”> spelling. Neither seems like progress to me.

	- Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150623/682707ed/attachment.html>