[PATCH] D15994: Allow for unfinished #if blocks in preambles.
This revision was automatically updated to reflect the committed changes. erikjv marked 3 inline comments as done. Closed by commit rL304207: Allow for unfinished #if blocks in preambles (authored by erikjv). Changed prior to commit: https://reviews.llvm.org/D15994?vs=98757=100691#toc Repository: rL LLVM https://reviews.llvm.org/D15994 Files: 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 cfe/trunk/test/Lexer/preamble2.c Index: cfe/trunk/lib/Lex/Preprocessor.cpp === --- cfe/trunk/lib/Lex/Preprocessor.cpp +++ cfe/trunk/lib/Lex/Preprocessor.cpp @@ -150,6 +150,9 @@ Ident_GetExceptionInfo = Ident_GetExceptionCode = nullptr; Ident_AbnormalTermination = nullptr; } + + if (this->PPOpts->GeneratePreamble) +PreambleConditionalStack.startRecording(); } Preprocessor::~Preprocessor() { @@ -532,6 +535,12 @@ // 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() { Index: cfe/trunk/lib/Lex/PPLexerChange.cpp === --- cfe/trunk/lib/Lex/PPLexerChange.cpp +++ cfe/trunk/lib/Lex/PPLexerChange.cpp @@ -46,6 +46,12 @@ }); } +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. Index: cfe/trunk/lib/Lex/Lexer.cpp === --- cfe/trunk/lib/Lex/Lexer.cpp +++ cfe/trunk/lib/Lex/Lexer.cpp @@ -550,8 +550,6 @@ enum PreambleDirectiveKind { PDK_Skipped, -PDK_StartIf, -PDK_EndIf, PDK_Unknown }; @@ -574,8 +572,6 @@ bool InPreprocessorDirective = false; Token TheTok; - Token IfStartTok; - unsigned IfCount = 0; SourceLocation ActiveCommentLoc; unsigned MaxLineOffset = 0; @@ -658,33 +654,18 @@ .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 @@ } 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 @@ 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. Index: cfe/trunk/lib/Frontend/ASTUnit.cpp === ---
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
rsmith accepted this revision. rsmith added a comment. Some comments, but I'm happy for you to go ahead and commit after addressing them. Thanks! Comment at: include/clang/Lex/Preprocessor.h:2004 + ArrayRef getPreambleConditionalStack() const + { return PreambleConditionalStack.getStack(); } + Please put the `{` on the previous line and the `}` on the next line. We don't use this "sandwich braces" style in clang except when defining the function on the same line as the declaration. Comment at: include/clang/Lex/PreprocessorLexer.h:182 + void setConditionalLevels(ArrayRef CL) + { +ConditionalStack.clear(); `{` on the previous line, please. Comment at: lib/Lex/Lexer.cpp:2548 + if (PP->isRecordingPreamble()) { +PP->setRecordedPreambleConditionalStack(ConditionalStack); +if (PP->isInPreamble()) This should be in the `isInPreamble()` conditional below, too. We don't want to make a redundant copy of the `ConditionalStack` at the end of each `#include`d file, just at the end of the overall preamble. Comment at: lib/Lex/PPLexerChange.cpp:49 +bool Preprocessor::isInPreamble() const { + if (IsFileLexer()) I think this would be better named as `isInMainFile`: we don't care whether we're in the preamble, or even if there is one, here (the caller checks that). https://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
nik added a comment. Ping?! https://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
erikjv updated this revision to Diff 98757. https://reviews.llvm.org/D15994 Files: include/clang/Lex/Preprocessor.h include/clang/Lex/PreprocessorLexer.h include/clang/Lex/PreprocessorOptions.h include/clang/Serialization/ASTBitCodes.h lib/Frontend/ASTUnit.cpp lib/Lex/Lexer.cpp lib/Lex/PPLexerChange.cpp lib/Lex/Preprocessor.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp test/Lexer/preamble.c test/Lexer/preamble2.c Index: test/Lexer/preamble2.c === --- /dev/null +++ test/Lexer/preamble2.c @@ -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. Index: test/Lexer/preamble.c === --- test/Lexer/preamble.c +++ test/Lexer/preamble.c @@ -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 @@ // CHECK-NEXT: #endif // CHECK-NEXT: #pragma unknown // CHECK-NEXT: #endif +// CHECK-NEXT: #ifdef WIBBLE +// CHECK-NEXT: #include "foo" // CHECK-NEXT: END. Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -1093,6 +1093,7 @@ 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 @@ Stream.EmitRecord(PP_COUNTER_VALUE, Record); } + if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) { +assert(!IsModule); +for (const auto : 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); Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -2925,6 +2925,21 @@ } break; +case PP_CONDITIONAL_STACK: + if (!Record.empty()) { +SmallVectorConditionalStack; +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]); Index: lib/Lex/Preprocessor.cpp === --- lib/Lex/Preprocessor.cpp +++ lib/Lex/Preprocessor.cpp @@ -150,6 +150,9 @@ Ident_GetExceptionInfo = Ident_GetExceptionCode = nullptr; Ident_AbnormalTermination = nullptr; } + + if (this->PPOpts->GeneratePreamble) +PreambleConditionalStack.startRecording(); } Preprocessor::~Preprocessor() { @@ -537,6 +540,12 @@ // 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() { Index: lib/Lex/PPLexerChange.cpp === --- lib/Lex/PPLexerChange.cpp +++ lib/Lex/PPLexerChange.cpp @@ -46,6 +46,12 @@ }); } +bool
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
erikjv marked 8 inline comments as done. erikjv added a comment. Fixed with the next patch. https://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
arphaman added a comment. Thanks for working on this! I have some comments below: Comment at: include/clang/Lex/Preprocessor.h:291 + +void setState(State s) { ConditionalStackState = s; } + `setState` is redundant IMHO, just assign to `ConditionalStackState` when you use `setState` below. Comment at: include/clang/Lex/Preprocessor.h:301 + +const ArrayRef getStack() const { + return ConditionalStack; the `const` is redundant for the return type. Comment at: include/clang/Lex/Preprocessor.h:310 + +void setStack(const ArrayRef ) { + if (!isRecording() && !isReplaying()) You can pass `ArrayRef` by value. Comment at: include/clang/Lex/Preprocessor.h:1974 + + const ArrayRef getPreambleConditionalStack() const + { return PreambleConditionalStack.getStack(); } the `const` is redundant for the return type. Comment at: include/clang/Lex/Preprocessor.h:1978 + void setRecordedPreambleConditionalStack( + const SmallVector) { +PreambleConditionalStack.setStack(s); Please use `ArrayRef` instead of `SmallVector` here and in the function below. Comment at: include/clang/Lex/PreprocessorLexer.h:181 + + void setConditionalLevels(const ArrayRef ) + { `ArrayRef` can be passed by value. Comment at: include/clang/Lex/PreprocessorOptions.h:87 + /// When the lexer is done, one of the things that need to be preserved is the + /// conditional #if stack, so the ASTWriter/ASTReader can safe/restore it when + /// processing the rest of the file. Typo: `save` Comment at: lib/Serialization/ASTReader.cpp:2896 +for (unsigned Idx = 0, N = Record.size() - 1; Idx < N; /* in loop */) { + auto loc = ReadSourceLocation(F, Record, Idx); + bool WasSkipping = Record[Idx++]; `Loc` should be capitalized. Comment at: lib/Serialization/ASTWriter.cpp:2269 + if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) { +for (const auto : PP.getPreambleConditionalStack()) { + AddSourceLocation(Cond.IfLoc, Record); I think you should add an assertion here that verifies that the ASTWriter isn't creating a module. https://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
erikjv updated this revision to Diff 87792. https://reviews.llvm.org/D15994 Files: include/clang/Lex/Preprocessor.h include/clang/Lex/PreprocessorLexer.h include/clang/Lex/PreprocessorOptions.h include/clang/Serialization/ASTBitCodes.h lib/Frontend/ASTUnit.cpp lib/Lex/Lexer.cpp lib/Lex/PPLexerChange.cpp lib/Lex/Preprocessor.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp test/Lexer/preamble.c test/Lexer/preamble2.c Index: test/Lexer/preamble2.c === --- /dev/null +++ test/Lexer/preamble2.c @@ -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. Index: test/Lexer/preamble.c === --- test/Lexer/preamble.c +++ test/Lexer/preamble.c @@ -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 @@ // CHECK-NEXT: #endif // CHECK-NEXT: #pragma unknown // CHECK-NEXT: #endif +// CHECK-NEXT: #ifdef WIBBLE +// CHECK-NEXT: #include "foo" // CHECK-NEXT: END. Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -1090,6 +1090,7 @@ 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); @@ -2264,6 +2265,17 @@ Stream.EmitRecord(PP_COUNTER_VALUE, Record); } + if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) { +for (const auto : 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); Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -2889,6 +2889,20 @@ } break; +case PP_CONDITIONAL_STACK: + if (!Record.empty()) { +SmallVectorConditionalStack; +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]); Index: lib/Lex/Preprocessor.cpp === --- lib/Lex/Preprocessor.cpp +++ lib/Lex/Preprocessor.cpp @@ -150,6 +150,9 @@ Ident_GetExceptionInfo = Ident_GetExceptionCode = nullptr; Ident_AbnormalTermination = nullptr; } + + if (this->PPOpts->GeneratePreamble) +PreambleConditionalStack.startRecording(); } Preprocessor::~Preprocessor() { @@ -537,6 +540,12 @@ // 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() { Index: lib/Lex/PPLexerChange.cpp === --- lib/Lex/PPLexerChange.cpp +++ lib/Lex/PPLexerChange.cpp @@ -46,6 +46,12 @@ }); } +bool Preprocessor::isInPreamble() const { + if (IsFileLexer()) +
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
rsmith added inline comments. Comment at: include/clang/Lex/Preprocessor.h:310 + +const Stack () const { + assert(ConditionalStack); Return an `ArrayRef` rather than exposing the type of the storage (which is an implementation detail), here and transitively through the callers of this. Comment at: include/clang/Lex/Preprocessor.h:322 + +void setStack(const Stack ) { + if (!isRecording() && !isReplaying()) Please pass in an `ArrayRef` here rather than a `SmallVector`. Comment at: include/clang/Lex/Preprocessor.h:328 + else +ConditionalStack = new Stack(s); +} I don't see a need to heap-allocate this separately from the `Preprocessor`. Comment at: include/clang/Lex/PreprocessorOptions.h:84 + + bool PreambleGeneration = false; Please add a doc comment for this option. Also, maybe `GeneratePreamble` to avoid ambiguity as to whether this is some kind of generation number for the preamble? Comment at: lib/Lex/PPLexerChange.cpp:139-142 + if (PreambleConditionalStack.isReplaying()) { +CurPPLexer->setConditionalLevels(PreambleConditionalStack.getStack()); +PreambleConditionalStack.doneReplaying(); + } I think this would make more sense two callers up, in `Preprocessor::EnterMainSourceFile` https://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
erikjv updated this revision to Diff 80589. https://reviews.llvm.org/D15994 Files: include/clang/Lex/Preprocessor.h include/clang/Lex/PreprocessorLexer.h include/clang/Lex/PreprocessorOptions.h include/clang/Serialization/ASTBitCodes.h lib/Frontend/ASTUnit.cpp lib/Lex/Lexer.cpp lib/Lex/PPLexerChange.cpp lib/Lex/Preprocessor.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp test/Lexer/preamble.c test/Lexer/preamble2.c Index: test/Lexer/preamble2.c === --- /dev/null +++ test/Lexer/preamble2.c @@ -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. Index: test/Lexer/preamble.c === --- test/Lexer/preamble.c +++ test/Lexer/preamble.c @@ -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 @@ // CHECK-NEXT: #endif // CHECK-NEXT: #pragma unknown // CHECK-NEXT: #endif +// CHECK-NEXT: #ifdef WIBBLE +// CHECK-NEXT: #include "foo" // CHECK-NEXT: END. Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -1071,6 +1071,7 @@ 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); @@ -2233,6 +2234,17 @@ Stream.EmitRecord(PP_COUNTER_VALUE, Record); } + if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) { +for (const auto : 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); Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -2870,6 +2870,20 @@ } break; +case PP_CONDITIONAL_STACK: + if (!Record.empty()) { +SmallVectorConditionalStack; +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]); Index: lib/Lex/Preprocessor.cpp === --- lib/Lex/Preprocessor.cpp +++ lib/Lex/Preprocessor.cpp @@ -150,6 +150,9 @@ Ident_GetExceptionInfo = Ident_GetExceptionCode = nullptr; Ident_AbnormalTermination = nullptr; } + + if (this->PPOpts->PreambleGeneration) +PreambleConditionalStack.startRecording(); } Preprocessor::~Preprocessor() { Index: lib/Lex/PPLexerChange.cpp === --- lib/Lex/PPLexerChange.cpp +++ lib/Lex/PPLexerChange.cpp @@ -46,6 +46,12 @@ }); } +bool Preprocessor::isInPreamble() const { + if (IsFileLexer()) +return IncludeMacroStack.size() == 1; + return false; +} + /// 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. @@ -129,6 +135,11 @@ Callbacks->FileChanged(CurLexer->getFileLoc(), PPCallbacks::EnterFile, FileType); } + + if
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
erikjv marked 7 inline comments as done. erikjv added inline comments. Comment at: lib/Serialization/ASTReader.cpp:2816 +} +PP.setReplayablePreambleConditionalStack(ConditionalStack); + } rsmith wrote: > Why can't we set the conditional stack on the `PPLexer` directly from here? > (Why do we need to store it separately?) Because there is no PPLexer yet. The deserialization is kicked off by ASTUnit::CodeComplete in Act->BeginSourceFile, while the lexer is created in Act->Execute. https://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
rsmith added inline comments. Comment at: include/clang/Lex/Preprocessor.h:283-286 + Off = 0, + Recording = 1, + Replaying = 2, + Done = 3 What's the difference between the `Off` and `Done` states? They seem to be the same to me? Comment at: include/clang/Lex/Preprocessor.h:317 +void setStack(const Stack ) +{ + if (!isRecording() && !isReplaying()) `{` on previous line. Comment at: include/clang/Lex/Preprocessor.h:326 + +bool HasRecordedPreamble() const { return ConditionalStack.getPointer(); } + Start function names with a lowercase letter. Comment at: include/clang/Lex/Preprocessor.h:329 + private: +llvm::PointerIntPair ConditionalStack; + } PreambleConditionalStack; We don't really care much about the `Preprocessor` object getting a few dozen bytes larger, since there will typically only be one of them. I don't think the complexity of a dynamically-allocated stack and a `PointerIntPair` is worthwhile here. Just store a `Stack` and a `State` directly as members. Comment at: include/clang/Lex/Preprocessor.h:1959-1963 + bool IsRecordingPreamble() const { +return PreambleConditionalStack.isRecording(); + } + + bool HasRecordedPreamble() const { Start function names with a lowercase letter. Comment at: lib/Lex/Lexer.cpp:622 StringRef Keyword = TheTok.getRawIdentifier(); PreambleDirectiveKind PDK = llvm::StringSwitch(Keyword) You can remove the special-case handling for `#if`/`#else`/`#endif` here. Comment at: lib/Lex/Lexer.cpp:2529-2532 + if (PP->IsRecordingPreamble()) { +PP->setRecordedPreambleConditionalStack(ConditionalStack); +ConditionalStack.clear(); + } We should only do this if we reach the end of the main source file. If we reach the end of a `#include`'d file with a `#if` still open, we should diagnose that. Comment at: lib/Serialization/ASTReader.cpp:2816 +} +PP.setReplayablePreambleConditionalStack(ConditionalStack); + } Why can't we set the conditional stack on the `PPLexer` directly from here? (Why do we need to store it separately?) https://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
erikjv added a comment. Yes, the patch was not committed, because there were errors in it. So, I fixed it and put up a new version, but I have no idea how to reset the state. Still waiting for rsmith to review... https://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15994: Allow for unfinished #if blocks in preambles.
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Looks like patch was not committed. https://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15994: Allow for unfinished #if blocks in preambles.
erikjv updated the summary for this revision. erikjv updated this revision to Diff 62748. erikjv added a comment. This version stores/loads the conditional stack in the preprocessor the the generated pch file. It suppresses errors about unfinished conditional preprocessor blocks, and after restoring, it will check if the block is closed in the main file. http://reviews.llvm.org/D15994 Files: include/clang/Lex/Preprocessor.h include/clang/Lex/PreprocessorLexer.h include/clang/Lex/PreprocessorOptions.h include/clang/Serialization/ASTBitCodes.h lib/Frontend/ASTUnit.cpp lib/Lex/Lexer.cpp lib/Lex/PPLexerChange.cpp lib/Lex/Preprocessor.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp test/Lexer/preamble.c test/Lexer/preamble2.c Index: test/Lexer/preamble2.c === --- /dev/null +++ test/Lexer/preamble2.c @@ -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. Index: test/Lexer/preamble.c === --- test/Lexer/preamble.c +++ test/Lexer/preamble.c @@ -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 @@ // CHECK-NEXT: #endif // CHECK-NEXT: #pragma unknown // CHECK-NEXT: #endif +// CHECK-NEXT: #ifdef WIBBLE +// CHECK-NEXT: #include "foo" // CHECK-NEXT: END. Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -983,6 +983,7 @@ RECORD(POINTERS_TO_MEMBERS_PRAGMA_OPTIONS); RECORD(UNUSED_LOCAL_TYPEDEF_NAME_CANDIDATES); RECORD(DELETE_EXPRS_TO_ANALYZE); + RECORD(PP_CONDITIONAL_STACK); // SourceManager Block. BLOCK(SOURCE_MANAGER_BLOCK); @@ -2140,6 +2141,17 @@ Stream.EmitRecord(PP_COUNTER_VALUE, Record); } + if (PP.IsRecordingPreamble() && PP.HasRecordedPreamble()) { +for (const auto : 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); Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -2803,6 +2803,20 @@ } break; +case PP_CONDITIONAL_STACK: + if (!Record.empty()) { +SmallVectorConditionalStack; +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]); Index: lib/Lex/Preprocessor.cpp === --- lib/Lex/Preprocessor.cpp +++ lib/Lex/Preprocessor.cpp @@ -140,6 +140,9 @@ Ident_GetExceptionInfo = Ident_GetExceptionCode = nullptr; Ident_AbnormalTermination = nullptr; } + + if (this->PPOpts->PreambleGeneration) +PreambleConditionalStack.startRecording(); } Preprocessor::~Preprocessor() { Index: lib/Lex/PPLexerChange.cpp === --- lib/Lex/PPLexerChange.cpp +++ lib/Lex/PPLexerChange.cpp @@ -130,6 +130,11 @@ Callbacks->FileChanged(CurLexer->getFileLoc(), PPCallbacks::EnterFile, FileType); } + + if
Re: [PATCH] D15994: Allow for unfinished #if blocks in preambles.
erikjv abandoned this revision. erikjv added a comment. This will give errors about unbalanced #if/#endif for header guards. http://reviews.llvm.org/D15994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15994: Allow for unfinished #if blocks in preambles.
erikjv created this revision. erikjv added a reviewer: bkramer. erikjv added a subscriber: cfe-commits. 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 makes code-completion very slow. This fixes PR26045. http://reviews.llvm.org/D15994 Files: lib/Lex/Lexer.cpp test/Lexer/preamble.c test/Lexer/preamble2.c Index: test/Lexer/preamble2.c === --- /dev/null +++ test/Lexer/preamble2.c @@ -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. Index: test/Lexer/preamble.c === --- test/Lexer/preamble.c +++ test/Lexer/preamble.c @@ -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 @@ // CHECK-NEXT: #endif // CHECK-NEXT: #pragma unknown // CHECK-NEXT: #endif +// CHECK-NEXT: #ifdef WIBBLE +// CHECK-NEXT: #include "foo" // CHECK-NEXT: END. Index: lib/Lex/Lexer.cpp === --- lib/Lex/Lexer.cpp +++ lib/Lex/Lexer.cpp @@ -552,7 +552,7 @@ bool InPreprocessorDirective = false; Token TheTok; Token IfStartTok; - unsigned IfCount = 0; + int IfCount = 0; SourceLocation ActiveCommentLoc; unsigned MaxLineOffset = 0; @@ -656,8 +656,10 @@ case PDK_EndIf: // Mismatched #endif. The preamble ends here. - if (IfCount == 0) + if (IfCount == 0) { +--IfCount; break; + } --IfCount; continue; @@ -682,7 +684,8 @@ } while (true); SourceLocation End; - if (IfCount) + if (IfCount < 0) // This allows for open #ifs, so a header with an include + // guard will have a preamble generated for it. End = IfStartTok.getLocation(); else if (ActiveCommentLoc.isValid()) End = ActiveCommentLoc; // don't truncate a decl comment. Index: test/Lexer/preamble2.c === --- /dev/null +++ test/Lexer/preamble2.c @@ -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. Index: test/Lexer/preamble.c === --- test/Lexer/preamble.c +++ test/Lexer/preamble.c @@ -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 @@ // CHECK-NEXT: #endif // CHECK-NEXT: #pragma unknown // CHECK-NEXT: #endif +// CHECK-NEXT: #ifdef WIBBLE +// CHECK-NEXT: #include "foo" // CHECK-NEXT: END. Index: lib/Lex/Lexer.cpp === --- lib/Lex/Lexer.cpp +++ lib/Lex/Lexer.cpp @@ -552,7 +552,7 @@ bool InPreprocessorDirective = false; Token TheTok; Token IfStartTok; - unsigned IfCount = 0; + int IfCount = 0; SourceLocation ActiveCommentLoc; unsigned MaxLineOffset = 0; @@ -656,8 +656,10 @@ case