r304207 - Allow for unfinished #if blocks in preambles

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 23 14:54:52 PDT 2017


On 23 June 2017 at 13:34, Benjamin Kramer via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> 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.


Does fixing the reversed condition I pointed out fix 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
> _______________________________________________
> 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/20170623/04784213/attachment-0001.html>


More information about the cfe-commits mailing list