r240156 - Introduced pragmas for audited nullability regions.

Aaron Ballman aaron at aaronballman.com
Mon Jun 22 11:53:42 PDT 2015


On Mon, Jun 22, 2015 at 2:15 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Jun 20, 2015, at 12:24 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>
> On Fri, Jun 19, 2015 at 2:25 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> Author: dgregor
> Date: Fri Jun 19 13:25:57 2015
> New Revision: 240156
>
> URL: http://llvm.org/viewvc/llvm-project?rev=240156&view=rev
> Log:
> Introduced pragmas for audited nullability regions.
>
> Introduce the clang pragmas "assume_nonnull begin" and "assume_nonnull
> end" in which we make default assumptions about the nullability of many
> unannotated pointers:
>
>  - Single-level pointers are inferred to __nonnull
>  - NSError** in a (function or method) parameter list is inferred to
>    NSError * __nullable * __nullable.
>  - CFErrorRef * in a (function or method) parameter list is inferred
>    to CFErrorRef __nullable * __nullable.
>  - Other multi-level pointers are never inferred to anything.
>
> Implements rdar://problem/19191042.
>
> Added:
>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-pragmas-1.h   (with props)
>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-pragmas-2.h   (with props)
>    cfe/trunk/test/SemaObjCXX/Inputs/nullability-pragmas-3.h   (with props)
>    cfe/trunk/test/SemaObjCXX/nullability-pragmas.mm
> Modified:
>    cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
>    cfe/trunk/include/clang/Lex/Preprocessor.h
>    cfe/trunk/include/clang/Parse/Parser.h
>    cfe/trunk/include/clang/Sema/DeclSpec.h
>    cfe/trunk/include/clang/Sema/Sema.h
>    cfe/trunk/lib/Lex/PPDirectives.cpp
>    cfe/trunk/lib/Lex/PPLexerChange.cpp
>    cfe/trunk/lib/Lex/PPMacroExpansion.cpp
>    cfe/trunk/lib/Lex/Pragma.cpp
>    cfe/trunk/lib/Parse/ParseObjc.cpp
>    cfe/trunk/lib/Sema/SemaExprObjC.cpp
>    cfe/trunk/lib/Sema/SemaObjCProperty.cpp
>    cfe/trunk/lib/Sema/SemaType.cpp
>    cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=240156&r1=240155&r2=240156&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Fri Jun 19 13:25:57
> 2015
> @@ -646,5 +646,20 @@ def warn_header_guard : Warning<
>   "%0 is used as a header guard here, followed by #define of a different
> macro">,
>   InGroup<DiagGroup<"header-guard">>;
> def note_header_guard : Note<
> -  "%0 is defined here; did you mean %1?">;
> +  "%0 is defined here; did you mean %1?">;
> +
> +let CategoryName = "Nullability Issue" in {
> +
> +def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
> +def err_pp_double_begin_of_assume_nonnull : Error<
> +  "already inside '#pragma clang assume_nonnull'">;
> +def err_pp_unmatched_end_of_assume_nonnull : Error<
> +  "not currently inside '#pragma clang assume_nonnull'">;
> +def err_pp_include_in_assume_nonnull : Error<
> +  "cannot #include files inside '#pragma clang assume_nonnull'">;
> +def err_pp_eof_in_assume_nonnull : Error<
> +  "'#pragma clang assume_nonnull' was not ended within this file">;
> +
> +}
> +
> }
>
> Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=240156&r1=240155&r2=240156&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
> +++ cfe/trunk/include/clang/Lex/Preprocessor.h Fri Jun 19 13:25:57 2015
> @@ -256,6 +256,10 @@ class Preprocessor : public RefCountedBa
>   /// \#pragma clang arc_cf_code_audited begin.
>   SourceLocation PragmaARCCFCodeAuditedLoc;
>
> +  /// \brief The source location of the currently-active
> +  /// \#pragma clang assume_nonnull begin.
> +  SourceLocation PragmaAssumeNonNullLoc;
> +
>   /// \brief True if we hit the code-completion point.
>   bool CodeCompletionReached;
>
> @@ -1250,6 +1254,20 @@ public:
>     PragmaARCCFCodeAuditedLoc = Loc;
>   }
>
> +  /// \brief The location of the currently-active \#pragma clang
> +  /// assume_nonnull begin.
> +  ///
> +  /// Returns an invalid location if there is no such pragma active.
> +  SourceLocation getPragmaAssumeNonNullLoc() const {
> +    return PragmaAssumeNonNullLoc;
> +  }
> +
> +  /// \brief Set the location of the currently-active \#pragma clang
> +  /// assume_nonnull begin.  An invalid location ends the pragma.
> +  void setPragmaAssumeNonNullLoc(SourceLocation Loc) {
> +    PragmaAssumeNonNullLoc = Loc;
> +  }
> +
>   /// \brief Set the directory in which the main file should be considered
>   /// to have been found, if it is not a real file.
>   void setMainFileDir(const DirectoryEntry *Dir) {
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=240156&r1=240155&r2=240156&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Fri Jun 19 13:25:57 2015
> @@ -139,11 +139,6 @@ class Parser : public CodeCompletionHand
>   // used as type traits.
>   llvm::SmallDenseMap<IdentifierInfo *, tok::TokenKind>
> RevertibleTypeTraits;
>
> -  /// Nullability type specifiers.
> -  IdentifierInfo *Ident___nonnull = nullptr;
> -  IdentifierInfo *Ident___nullable = nullptr;
> -  IdentifierInfo *Ident___null_unspecified = nullptr;
> -
>   std::unique_ptr<PragmaHandler> AlignHandler;
>   std::unique_ptr<PragmaHandler> GCCVisibilityHandler;
>   std::unique_ptr<PragmaHandler> OptionsHandler;
> @@ -308,9 +303,11 @@ public:
>     return true;
>   }
>
> -  /// Retrieve the underscored keyword (__nonnull, __nullable,
> -  /// __null_unspecified) that corresponds to the given nullability kind.
> -  IdentifierInfo *getNullabilityKeyword(NullabilityKind nullability);
> +  /// Retrieve the underscored keyword (__nonnull, __nullable) that
> corresponds
> +  /// to the given nullability kind.
> +  IdentifierInfo *getNullabilityKeyword(NullabilityKind nullability) {
> +    return Actions.getNullabilityKeyword(nullability);
> +  }
>
> private:
>
> //===--------------------------------------------------------------------===//
>
> Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=240156&r1=240155&r2=240156&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
> +++ cfe/trunk/include/clang/Sema/DeclSpec.h Fri Jun 19 13:25:57 2015
> @@ -1650,7 +1650,13 @@ private:
>   bool InlineParamsUsed;
>
>   /// \brief true if the declaration is preceded by \c __extension__.
> -  bool Extension : 1;
> +  unsigned Extension : 1;
> +
> +  /// Indicates whether this is an Objective-C instance variable.
> +  unsigned ObjCIvar : 1;
> +
> +  /// Indicates whether this is an Objective-C 'weak' property.
> +  unsigned ObjCWeakProperty : 1;
>
>   /// \brief If this is the second or subsequent declarator in this
> declaration,
>   /// the location of the comma before this declarator.
> @@ -1669,7 +1675,8 @@ public:
>       GroupingParens(false), FunctionDefinition(FDK_Declaration),
>       Redeclaration(false),
>       Attrs(ds.getAttributePool().getFactory()), AsmLabel(nullptr),
> -      InlineParamsUsed(false), Extension(false) {
> +      InlineParamsUsed(false), Extension(false), ObjCIvar(false),
> +      ObjCWeakProperty(false) {
>   }
>
>   ~Declarator() {
> @@ -1747,6 +1754,8 @@ public:
>     Attrs.clear();
>     AsmLabel = nullptr;
>     InlineParamsUsed = false;
> +    ObjCIvar = false;
> +    ObjCWeakProperty = false;
>     CommaLoc = SourceLocation();
>     EllipsisLoc = SourceLocation();
>   }
> @@ -2155,6 +2164,12 @@ public:
>   void setExtension(bool Val = true) { Extension = Val; }
>   bool getExtension() const { return Extension; }
>
> +  void setObjCIvar(bool Val = true) { ObjCIvar = Val; }
> +  bool isObjCIvar() const { return ObjCIvar; }
> +
> +  void setObjCWeakProperty(bool Val = true) { ObjCWeakProperty = Val; }
> +  bool isObjCWeakProperty() const { return ObjCWeakProperty; }
> +
>   void setInvalidType(bool Val = true) { InvalidType = Val; }
>   bool isInvalidType() const {
>     return InvalidType || DS.getTypeSpecType() == DeclSpec::TST_error;
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=240156&r1=240155&r2=240156&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Fri Jun 19 13:25:57 2015
> @@ -1157,6 +1157,16 @@ public:
>
>   bool CheckFunctionReturnType(QualType T, SourceLocation Loc);
>
> +  unsigned deduceWeakPropertyFromType(QualType T) {
> +    if ((getLangOpts().getGC() != LangOptions::NonGC &&
> +         T.isObjCGCWeak()) ||
> +        (getLangOpts().ObjCAutoRefCount &&
> +         T.getObjCLifetime() == Qualifiers::OCL_Weak))
> +        return ObjCDeclSpec::DQ_PR_weak;
> +    return 0;
> +  }
> +
> +
>   /// \brief Build a function type.
>   ///
>   /// This routine checks the function type according to C++ rules and
> @@ -8782,6 +8792,13 @@ private:
>   mutable IdentifierInfo *Ident_super;
>   mutable IdentifierInfo *Ident___float128;
>
> +  /// Nullability type specifiers.
> +  IdentifierInfo *Ident___nonnull = nullptr;
> +  IdentifierInfo *Ident___nullable = nullptr;
> +  IdentifierInfo *Ident___null_unspecified = nullptr;
> +
> +  IdentifierInfo *Ident_NSError = nullptr;
> +
> protected:
>   friend class Parser;
>   friend class InitializationSequence;
> @@ -8790,6 +8807,15 @@ protected:
>   friend class ASTWriter;
>
> public:
> +  /// Retrieve the keyword associated
> +  IdentifierInfo *getNullabilityKeyword(NullabilityKind nullability);
> +
> +  /// The struct behind the CFErrorRef pointer.
> +  RecordDecl *CFError = nullptr;
> +
> +  /// Retrieve the identifier "NSError".
> +  IdentifierInfo *getNSErrorIdent();
> +
>   /// \brief Retrieve the parser's current scope.
>   ///
>   /// This routine must only be used when it is certain that semantic
> analysis
>
> Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=240156&r1=240155&r2=240156&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
> +++ cfe/trunk/lib/Lex/PPDirectives.cpp Fri Jun 19 13:25:57 2015
> @@ -1575,6 +1575,15 @@ void Preprocessor::HandleIncludeDirectiv
>     PragmaARCCFCodeAuditedLoc = SourceLocation();
>   }
>
> +  // Complain about attempts to #include files in an assume-nonnull pragma.
> +  if (PragmaAssumeNonNullLoc.isValid()) {
> +    Diag(HashLoc, diag::err_pp_include_in_assume_nonnull);
> +    Diag(PragmaAssumeNonNullLoc, diag::note_pragma_entered_here);
> +
> +    // Immediately leave the pragma.
> +    PragmaAssumeNonNullLoc = SourceLocation();
> +  }
> +
>   if (HeaderInfo.HasIncludeAliasMap()) {
>     // Map the filename with the brackets still attached.  If the name
> doesn't
>     // map to anything, fall back on the filename we've already gotten the
>
> Modified: cfe/trunk/lib/Lex/PPLexerChange.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPLexerChange.cpp?rev=240156&r1=240155&r2=240156&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPLexerChange.cpp (original)
> +++ cfe/trunk/lib/Lex/PPLexerChange.cpp Fri Jun 19 13:25:57 2015
> @@ -355,6 +355,17 @@ bool Preprocessor::HandleEndOfFile(Token
>     PragmaARCCFCodeAuditedLoc = SourceLocation();
>   }
>
> +  // Complain about reaching a true EOF within assume_nonnull.
> +  // We don't want to complain about reaching the end of a macro
> +  // instantiation or a _Pragma.
> +  if (PragmaAssumeNonNullLoc.isValid() &&
> +      !isEndOfMacro && !(CurLexer && CurLexer->Is_PragmaLexer)) {
> +    Diag(PragmaAssumeNonNullLoc, diag::err_pp_eof_in_assume_nonnull);
> +
> +    // Recover by leaving immediately.
> +    PragmaAssumeNonNullLoc = SourceLocation();
> +  }
> +
>   // If this is a #include'd file, pop it off the include stack and continue
>   // lexing the #includer file.
>   if (!IncludeMacroStack.empty()) {
>
> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=240156&r1=240155&r2=240156&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Fri Jun 19 13:25:57 2015
> @@ -1052,6 +1052,7 @@ static bool HasFeature(const Preprocesso
>       .Case("address_sanitizer",
>             LangOpts.Sanitize.hasOneOf(SanitizerKind::Address |
>                                        SanitizerKind::KernelAddress))
> +      .Case("assume_nonnull", LangOpts.ObjC1 || LangOpts.GNUMode)
>       .Case("attribute_analyzer_noreturn", true)
>       .Case("attribute_availability", true)
>       .Case("attribute_availability_with_message", true)
>
> Modified: cfe/trunk/lib/Lex/Pragma.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Pragma.cpp?rev=240156&r1=240155&r2=240156&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/Pragma.cpp (original)
> +++ cfe/trunk/lib/Lex/Pragma.cpp Fri Jun 19 13:25:57 2015
> @@ -1342,6 +1342,60 @@ struct PragmaARCCFCodeAuditedHandler : p
>   }
> };
>
> +/// PragmaAssumeNonNullHandler -
> +///   \#pragma clang assume_nonnull begin/end
> +struct PragmaAssumeNonNullHandler : public PragmaHandler {
> +  PragmaAssumeNonNullHandler() : PragmaHandler("assume_nonnull") {}
> +  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
> +                    Token &NameTok) override {
> +    SourceLocation Loc = NameTok.getLocation();
> +    bool IsBegin;
> +
> +    Token Tok;
> +
> +    // Lex the 'begin' or 'end'.
> +    PP.LexUnexpandedToken(Tok);
> +    const IdentifierInfo *BeginEnd = Tok.getIdentifierInfo();
> +    if (BeginEnd && BeginEnd->isStr("begin")) {
> +      IsBegin = true;
>
>
> Elide braces (here and elsewhere).
>
>
> FWIW, I find it hideous to elide braces in some parts of an if-else chain
> and not others.

As do I (despite that seeming to be part of our coding style
guidelines). I hadn't noticed that this was an instance of that,
however. Feel free to ignore for such places. ;-)

>
>
> +
> +/// Classify the given declarator, whose type-specified is \c type, based
> on
> +/// what kind of pointer it refers to.
> +///
> +/// This is used to determine the default nullability.
> +static PointerDeclaratorKind classifyPointerDeclarator(Sema &S,
> +                                                       QualType type,
> +                                                       Declarator
> &declarator) {
> +  unsigned numNormalPointers = 0;
> +
> +  // For any dependent type, we consider it a non-pointer.
> +  if (type->isDependentType())
> +    return PointerDeclaratorKind::NonPointer;
> +
> +  // Look through the declarator chunks to identify pointers.
> +  for (unsigned i = 0, n = declarator.getNumTypeObjects(); i != n; ++i) {
> +    DeclaratorChunk &chunk = declarator.getTypeObject(i);
> +    switch (chunk.Kind) {
> +    case DeclaratorChunk::Array:
> +    case DeclaratorChunk::Function:
> +      break;
> +
> +    case DeclaratorChunk::BlockPointer:
> +    case DeclaratorChunk::MemberPointer:
> +      return numNormalPointers > 0 ?
> PointerDeclaratorKind::MultiLevelPointer
> +                                   :
> PointerDeclaratorKind::SingleLevelPointer;
> +
> +    case DeclaratorChunk::Paren:
> +    case DeclaratorChunk::Reference:
> +      continue;
> +
> +    case DeclaratorChunk::Pointer:
> +      ++numNormalPointers;
> +      if (numNormalPointers > 2)
>
>
> Why > instead of >=?
>
>
> Because of the way we deal with the type-specifier being a pointer (via a
> typedef), below.

Ah, thank you, that was confusing me!

~Aaron

>
> - Doug
>



More information about the cfe-commits mailing list