r304207 - Allow for unfinished #if blocks in preambles

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 23 13:34:52 PDT 2017


Since this change went in I'm seeing spurious errors whenever editing
a header file, filed https://bugs.llvm.org/show_bug.cgi?id=33574 for
that.

On Tue, May 30, 2017 at 1:54 PM, Erik Verbruggen via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: erikjv
> Date: Tue May 30 06:54:55 2017
> New Revision: 304207
>
> URL: http://llvm.org/viewvc/llvm-project?rev=304207&view=rev
> Log:
> Allow for unfinished #if blocks in preambles
>
> Previously, a preamble only included #if blocks (and friends like
> ifdef) if there was a corresponding #endif before any declaration or
> definition. The problem is that any header file that uses include guards
> will not have a preamble generated, which can make code-completion very
> slow.
>
> To prevent errors about unbalanced preprocessor conditionals in the
> preamble, and unbalanced preprocessor conditionals after a preamble
> containing unfinished conditionals, the conditional stack is stored
> in the pch file.
>
> This fixes PR26045.
>
> Differential Revision: http://reviews.llvm.org/D15994
>
>
> Added:
>     cfe/trunk/test/Lexer/preamble2.c
> Modified:
>     cfe/trunk/include/clang/Lex/Preprocessor.h
>     cfe/trunk/include/clang/Lex/PreprocessorLexer.h
>     cfe/trunk/include/clang/Lex/PreprocessorOptions.h
>     cfe/trunk/include/clang/Serialization/ASTBitCodes.h
>     cfe/trunk/lib/Frontend/ASTUnit.cpp
>     cfe/trunk/lib/Lex/Lexer.cpp
>     cfe/trunk/lib/Lex/PPLexerChange.cpp
>     cfe/trunk/lib/Lex/Preprocessor.cpp
>     cfe/trunk/lib/Serialization/ASTReader.cpp
>     cfe/trunk/lib/Serialization/ASTWriter.cpp
>     cfe/trunk/test/Lexer/preamble.c
>
> Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
> +++ cfe/trunk/include/clang/Lex/Preprocessor.h Tue May 30 06:54:55 2017
> @@ -283,6 +283,44 @@ class Preprocessor {
>    /// This is used when loading a precompiled preamble.
>    std::pair<int, bool> SkipMainFilePreamble;
>
> +  class PreambleConditionalStackStore {
> +    enum State {
> +      Off = 0,
> +      Recording = 1,
> +      Replaying = 2,
> +    };
> +
> +  public:
> +    PreambleConditionalStackStore() : ConditionalStackState(Off) {}
> +
> +    void startRecording() { ConditionalStackState = Recording; }
> +    void startReplaying() { ConditionalStackState = Replaying; }
> +    bool isRecording() const { return ConditionalStackState == Recording; }
> +    bool isReplaying() const { return ConditionalStackState == Replaying; }
> +
> +    ArrayRef<PPConditionalInfo> getStack() const {
> +      return ConditionalStack;
> +    }
> +
> +    void doneReplaying() {
> +      ConditionalStack.clear();
> +      ConditionalStackState = Off;
> +    }
> +
> +    void setStack(ArrayRef<PPConditionalInfo> s) {
> +      if (!isRecording() && !isReplaying())
> +        return;
> +      ConditionalStack.clear();
> +      ConditionalStack.append(s.begin(), s.end());
> +    }
> +
> +    bool hasRecordedPreamble() const { return !ConditionalStack.empty(); }
> +
> +  private:
> +    SmallVector<PPConditionalInfo, 4> ConditionalStack;
> +    State ConditionalStackState;
> +  } PreambleConditionalStack;
> +
>    /// \brief The current top of the stack that we're lexing from if
>    /// not expanding a macro and we are lexing directly from source code.
>    ///
> @@ -1695,6 +1733,11 @@ public:
>    /// \brief Return true if we're in the top-level file, not in a \#include.
>    bool isInPrimaryFile() const;
>
> +  /// \brief Return true if we're in the main file (specifically, if we are 0
> +  /// (zero) levels deep \#include. This is used by the lexer to determine if
> +  /// it needs to generate errors about unterminated \#if directives.
> +  bool isInMainFile() const;
> +
>    /// \brief Handle cases where the \#include name is expanded
>    /// from a macro as multiple tokens, which need to be glued together.
>    ///
> @@ -1932,6 +1975,27 @@ public:
>                                                            Module *M,
>                                                            SourceLocation MLoc);
>
> +  bool isRecordingPreamble() const {
> +    return PreambleConditionalStack.isRecording();
> +  }
> +
> +  bool hasRecordedPreamble() const {
> +    return PreambleConditionalStack.hasRecordedPreamble();
> +  }
> +
> +  ArrayRef<PPConditionalInfo> getPreambleConditionalStack() const {
> +      return PreambleConditionalStack.getStack();
> +  }
> +
> +  void setRecordedPreambleConditionalStack(ArrayRef<PPConditionalInfo> s) {
> +    PreambleConditionalStack.setStack(s);
> +  }
> +
> +  void setReplayablePreambleConditionalStack(ArrayRef<PPConditionalInfo> s) {
> +    PreambleConditionalStack.startReplaying();
> +    PreambleConditionalStack.setStack(s);
> +  }
> +
>  private:
>    // Macro handling.
>    void HandleDefineDirective(Token &Tok, bool ImmediatelyAfterTopLevelIfndef);
>
> Modified: cfe/trunk/include/clang/Lex/PreprocessorLexer.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/PreprocessorLexer.h?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/PreprocessorLexer.h (original)
> +++ cfe/trunk/include/clang/Lex/PreprocessorLexer.h Tue May 30 06:54:55 2017
> @@ -17,6 +17,7 @@
>
>  #include "clang/Lex/MultipleIncludeOpt.h"
>  #include "clang/Lex/Token.h"
> +#include "llvm/ADT/ArrayRef.h"
>  #include "llvm/ADT/SmallVector.h"
>
>  namespace clang {
> @@ -176,6 +177,11 @@ public:
>    conditional_iterator conditional_end() const {
>      return ConditionalStack.end();
>    }
> +
> +  void setConditionalLevels(ArrayRef<PPConditionalInfo> CL) {
> +    ConditionalStack.clear();
> +    ConditionalStack.append(CL.begin(), CL.end());
> +  }
>  };
>
>  }  // end namespace clang
>
> Modified: cfe/trunk/include/clang/Lex/PreprocessorOptions.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/PreprocessorOptions.h?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/PreprocessorOptions.h (original)
> +++ cfe/trunk/include/clang/Lex/PreprocessorOptions.h Tue May 30 06:54:55 2017
> @@ -80,7 +80,14 @@ public:
>    /// The boolean indicates whether the preamble ends at the start of a new
>    /// line.
>    std::pair<unsigned, bool> PrecompiledPreambleBytes;
> -
> +
> +  /// \brief True indicates that a preamble is being generated.
> +  ///
> +  /// When the lexer is done, one of the things that need to be preserved is the
> +  /// conditional #if stack, so the ASTWriter/ASTReader can save/restore it when
> +  /// processing the rest of the file.
> +  bool GeneratePreamble;
> +
>    /// The implicit PTH input included at the start of the translation unit, or
>    /// empty.
>    std::string ImplicitPTHInclude;
> @@ -144,6 +151,7 @@ public:
>                            AllowPCHWithCompilerErrors(false),
>                            DumpDeserializedPCHDecls(false),
>                            PrecompiledPreambleBytes(0, true),
> +                          GeneratePreamble(false),
>                            RemappedFilesKeepOriginalName(true),
>                            RetainRemappedFileBuffers(false),
>                            ObjCXXARCStandardLibrary(ARCXX_nolib) { }
>
> Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Tue May 30 06:54:55 2017
> @@ -607,6 +607,9 @@ namespace clang {
>
>        /// \brief Record code for \#pragma pack options.
>        PACK_PRAGMA_OPTIONS = 61,
> +
> +      /// \brief The stack of open #ifs/#ifdefs recorded in a preamble.
> +      PP_CONDITIONAL_STACK = 62,
>      };
>
>      /// \brief Record types used within a source manager block.
>
> Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
> +++ cfe/trunk/lib/Frontend/ASTUnit.cpp Tue May 30 06:54:55 2017
> @@ -1999,6 +1999,7 @@ ASTUnit *ASTUnit::LoadFromCommandLine(
>    PreprocessorOptions &PPOpts = CI->getPreprocessorOpts();
>    PPOpts.RemappedFilesKeepOriginalName = RemappedFilesKeepOriginalName;
>    PPOpts.AllowPCHWithCompilerErrors = AllowPCHWithCompilerErrors;
> +  PPOpts.GeneratePreamble = PrecompilePreambleAfterNParses != 0;
>
>    // Override the resources path.
>    CI->getHeaderSearchOpts().ResourceDir = ResourceFilesPath;
>
> Modified: cfe/trunk/lib/Lex/Lexer.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/Lexer.cpp (original)
> +++ cfe/trunk/lib/Lex/Lexer.cpp Tue May 30 06:54:55 2017
> @@ -550,8 +550,6 @@ namespace {
>
>    enum PreambleDirectiveKind {
>      PDK_Skipped,
> -    PDK_StartIf,
> -    PDK_EndIf,
>      PDK_Unknown
>    };
>
> @@ -574,8 +572,6 @@ std::pair<unsigned, bool> Lexer::Compute
>
>    bool InPreprocessorDirective = false;
>    Token TheTok;
> -  Token IfStartTok;
> -  unsigned IfCount = 0;
>    SourceLocation ActiveCommentLoc;
>
>    unsigned MaxLineOffset = 0;
> @@ -658,33 +654,18 @@ std::pair<unsigned, bool> Lexer::Compute
>                .Case("sccs", PDK_Skipped)
>                .Case("assert", PDK_Skipped)
>                .Case("unassert", PDK_Skipped)
> -              .Case("if", PDK_StartIf)
> -              .Case("ifdef", PDK_StartIf)
> -              .Case("ifndef", PDK_StartIf)
> +              .Case("if", PDK_Skipped)
> +              .Case("ifdef", PDK_Skipped)
> +              .Case("ifndef", PDK_Skipped)
>                .Case("elif", PDK_Skipped)
>                .Case("else", PDK_Skipped)
> -              .Case("endif", PDK_EndIf)
> +              .Case("endif", PDK_Skipped)
>                .Default(PDK_Unknown);
>
>          switch (PDK) {
>          case PDK_Skipped:
>            continue;
>
> -        case PDK_StartIf:
> -          if (IfCount == 0)
> -            IfStartTok = HashTok;
> -
> -          ++IfCount;
> -          continue;
> -
> -        case PDK_EndIf:
> -          // Mismatched #endif. The preamble ends here.
> -          if (IfCount == 0)
> -            break;
> -
> -          --IfCount;
> -          continue;
> -
>          case PDK_Unknown:
>            // We don't know what this directive is; stop at the '#'.
>            break;
> @@ -705,16 +686,13 @@ std::pair<unsigned, bool> Lexer::Compute
>    } while (true);
>
>    SourceLocation End;
> -  if (IfCount)
> -    End = IfStartTok.getLocation();
> -  else if (ActiveCommentLoc.isValid())
> +  if (ActiveCommentLoc.isValid())
>      End = ActiveCommentLoc; // don't truncate a decl comment.
>    else
>      End = TheTok.getLocation();
>
>    return std::make_pair(End.getRawEncoding() - StartLoc.getRawEncoding(),
> -                        IfCount? IfStartTok.isAtStartOfLine()
> -                               : TheTok.isAtStartOfLine());
> +                        TheTok.isAtStartOfLine());
>  }
>
>  /// AdvanceToTokenCharacter - Given a location that specifies the start of a
> @@ -2570,6 +2548,11 @@ bool Lexer::LexEndOfFile(Token &Result,
>      return true;
>    }
>
> +  if (PP->isRecordingPreamble() && !PP->isInMainFile()) {
> +    PP->setRecordedPreambleConditionalStack(ConditionalStack);
> +    ConditionalStack.clear();
> +  }
> +
>    // Issue diagnostics for unterminated #if and missing newline.
>
>    // If we are in a #if directive, emit an error.
>
> Modified: cfe/trunk/lib/Lex/PPLexerChange.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPLexerChange.cpp?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPLexerChange.cpp (original)
> +++ cfe/trunk/lib/Lex/PPLexerChange.cpp Tue May 30 06:54:55 2017
> @@ -46,6 +46,12 @@ bool Preprocessor::isInPrimaryFile() con
>    });
>  }
>
> +bool Preprocessor::isInMainFile() const {
> +  if (IsFileLexer())
> +    return IncludeMacroStack.size() == 0;
> +  return true;
> +}
> +
>  /// getCurrentLexer - Return the current file lexer being lexed from.  Note
>  /// that this ignores any potentially active macro expansions and _Pragma
>  /// expansions going on at the time.
>
> Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
> +++ cfe/trunk/lib/Lex/Preprocessor.cpp Tue May 30 06:54:55 2017
> @@ -150,6 +150,9 @@ Preprocessor::Preprocessor(std::shared_p
>      Ident_GetExceptionInfo = Ident_GetExceptionCode = nullptr;
>      Ident_AbnormalTermination = nullptr;
>    }
> +
> +  if (this->PPOpts->GeneratePreamble)
> +    PreambleConditionalStack.startRecording();
>  }
>
>  Preprocessor::~Preprocessor() {
> @@ -532,6 +535,12 @@ void Preprocessor::EnterMainSourceFile()
>
>    // Start parsing the predefines.
>    EnterSourceFile(FID, nullptr, SourceLocation());
> +
> +  // Restore the conditional stack from the preamble, if there is one.
> +  if (PreambleConditionalStack.isReplaying()) {
> +    CurPPLexer->setConditionalLevels(PreambleConditionalStack.getStack());
> +    PreambleConditionalStack.doneReplaying();
> +  }
>  }
>
>  void Preprocessor::EndSourceFile() {
>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue May 30 06:54:55 2017
> @@ -2925,6 +2925,21 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
>        }
>        break;
>
> +    case PP_CONDITIONAL_STACK:
> +      if (!Record.empty()) {
> +        SmallVector<PPConditionalInfo, 4> ConditionalStack;
> +        for (unsigned Idx = 0, N = Record.size() - 1; Idx < N; /* in loop */) {
> +          auto Loc = ReadSourceLocation(F, Record, Idx);
> +          bool WasSkipping = Record[Idx++];
> +          bool FoundNonSkip = Record[Idx++];
> +          bool FoundElse = Record[Idx++];
> +          ConditionalStack.push_back(
> +              {Loc, WasSkipping, FoundNonSkip, FoundElse});
> +        }
> +        PP.setReplayablePreambleConditionalStack(ConditionalStack);
> +      }
> +      break;
> +
>      case PP_COUNTER_VALUE:
>        if (!Record.empty() && Listener)
>          Listener->ReadCounter(F, Record[0]);
>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue May 30 06:54:55 2017
> @@ -1093,6 +1093,7 @@ void ASTWriter::WriteBlockInfoBlock() {
>    RECORD(UNUSED_LOCAL_TYPEDEF_NAME_CANDIDATES);
>    RECORD(DELETE_EXPRS_TO_ANALYZE);
>    RECORD(CUDA_PRAGMA_FORCE_HOST_DEVICE_DEPTH);
> +  RECORD(PP_CONDITIONAL_STACK);
>
>    // SourceManager Block.
>    BLOCK(SOURCE_MANAGER_BLOCK);
> @@ -2302,6 +2303,18 @@ void ASTWriter::WritePreprocessor(const
>      Stream.EmitRecord(PP_COUNTER_VALUE, Record);
>    }
>
> +  if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) {
> +    assert(!IsModule);
> +    for (const auto &Cond : PP.getPreambleConditionalStack()) {
> +      AddSourceLocation(Cond.IfLoc, Record);
> +      Record.push_back(Cond.WasSkipping);
> +      Record.push_back(Cond.FoundNonSkip);
> +      Record.push_back(Cond.FoundElse);
> +    }
> +    Stream.EmitRecord(PP_CONDITIONAL_STACK, Record);
> +    Record.clear();
> +  }
> +
>    // Enter the preprocessor block.
>    Stream.EnterSubblock(PREPROCESSOR_BLOCK_ID, 3);
>
>
> Modified: cfe/trunk/test/Lexer/preamble.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/preamble.c?rev=304207&r1=304206&r2=304207&view=diff
> ==============================================================================
> --- cfe/trunk/test/Lexer/preamble.c (original)
> +++ cfe/trunk/test/Lexer/preamble.c Tue May 30 06:54:55 2017
> @@ -9,15 +9,12 @@
>  #pragma unknown
>  #endif
>  #ifdef WIBBLE
> -#include "honk"
> -#else
> -int foo();
> +#include "foo"
> +int bar;
>  #endif
>
>  // This test checks for detection of the preamble of a file, which
> -// includes all of the starting comments and #includes. Note that any
> -// changes to the preamble part of this file must be mirrored in
> -// Inputs/preamble.txt, since we diff against it.
> +// includes all of the starting comments and #includes.
>
>  // RUN: %clang_cc1 -print-preamble %s > %t
>  // RUN: echo END. >> %t
> @@ -33,4 +30,6 @@ int foo();
>  // CHECK-NEXT: #endif
>  // CHECK-NEXT: #pragma unknown
>  // CHECK-NEXT: #endif
> +// CHECK-NEXT: #ifdef WIBBLE
> +// CHECK-NEXT: #include "foo"
>  // CHECK-NEXT: END.
>
> Added: cfe/trunk/test/Lexer/preamble2.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/preamble2.c?rev=304207&view=auto
> ==============================================================================
> --- cfe/trunk/test/Lexer/preamble2.c (added)
> +++ cfe/trunk/test/Lexer/preamble2.c Tue May 30 06:54:55 2017
> @@ -0,0 +1,19 @@
> +// Preamble detection test: header with an include guard.
> +#ifndef HEADER_H
> +#define HEADER_H
> +#include "foo"
> +int bar;
> +#endif
> +
> +// This test checks for detection of the preamble of a file, which
> +// includes all of the starting comments and #includes.
> +
> +// RUN: %clang_cc1 -print-preamble %s > %t
> +// RUN: echo END. >> %t
> +// RUN: FileCheck < %t %s
> +
> +// CHECK: // Preamble detection test: header with an include guard.
> +// CHECK-NEXT: #ifndef HEADER_H
> +// CHECK-NEXT: #define HEADER_H
> +// CHECK-NEXT: #include "foo"
> +// CHECK-NEXT: END.
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


More information about the cfe-commits mailing list