r304207 - Allow for unfinished #if blocks in preambles

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue May 30 11:59:24 PDT 2017


On 30 May 2017 at 04:54, 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()) {
>

You've picked up a bogus ! on the second condition between the review and
the commit. We want to store and clear the preamble when we get to the end
of the main file, not when we get to the end of every file other than the
main file.


> +    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;
> +}
>

Looks like Preprocessor::isInPrimaryFile already exists and does the same
thing (but handles some corner cases better).


> +
>  /// 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170530/e2c31cb7/attachment-0001.html>


More information about the cfe-commits mailing list