[Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Hahnfeld]

Summary

This new method repeatedly calls Lex() until end of file is reached and optionally fills a std::vector of Tokens. Use it in Clang's unit tests to avoid quite some code duplication.

[Hahnfeld]

ping :)

[Hahnfeld]

ping!

[v.g.vassilev]

clang/lib/Lex/Preprocessor.cpp

1001

Shouldn't we take the results as a LexAll(std::vector<Token> &result)?

Perhaps we should rename this interface to LexTokensUntilEOF?

[aaron.ballman]

clang/lib/Lex/Preprocessor.cpp

1001

I think both suggestions make sense, especially given this doesn't return the EOF token (so it doesn't really lex *all*). Though, given how many callers do not want to store the tokens, perhaps it should take a std::optional or a pointer to the vector so we can skip the storage work if it's unnecessary?

1003

I'd prefer not to assume the token stream has an EOF token (perhaps the stream is one only being used to parse until the eod token instead), so if we can turn this into a non-infinite loop, that would make me more comfortable.

[Hahnfeld]

Rename to Preprocessor::LexTokensUntilEOF()

[Hahnfeld]

clang/lib/Lex/Preprocessor.cpp

1001

The new LexTokensUntilEOF now takes an optional std::vector<Token> *Tokens = nullptr.

1003

I'm not sure I understand entirely. Do you want something like

tok.isOneOf(tok::unknown, tok::eof, tok::eod)

instead of tok.is(tok::eof)? Can this happen at the level of the Preprocessor?

[aaron.ballman]

clang/lib/Lex/Preprocessor.cpp

1003

I was thinking something more along the lines of:

if (Tokens) {
  for (Token Tok; !Tok.isOneOf(tok::eof, tok::eod); Lex(Tok))
    Tokens->push_back(Tok);
}

but I hadn't thought about tok::unknown; that might be a good one to also include given that clangd operates on partial sources.

[v.g.vassilev]

clang/lib/Lex/Preprocessor.cpp

1003

I was wondering if we could somehow merge this routine with Parser::SkipUntil since they seem to be doing a very similar tasks.

[aaron.ballman]

clang/lib/Lex/Preprocessor.cpp

1003

That could perhaps end up looking reasonable (they do similar tasks aside from collecting the tokens that are being skipped). Do you need the interface to be on Preprocessor or Parser though (or does it not really matter for you)?

[Hahnfeld]

Handle all of tok::unknown, tok::eof, tok::eod.

clang/lib/Lex/Preprocessor.cpp

1003

tok.isOneOf(tok::unknown, tok::eof, tok::eod)

I implemented this check, let me know if this looks reasonable. The code you posted doesn't do what we need because we also want to lex if Tokens is nullptr, so the hierarchy must be an if inside the loop.
Given these additional token kinds, does UntilEOF still make sense or do we want another name? Note that I'll leave repl_input_end to https://reviews.llvm.org/D158415.

I was wondering if we could somehow merge this routine with Parser::SkipUntil since they seem to be doing a very similar tasks.

I'm not sure this makes sense, given that Parser::SkipUntil requires some knowledge about the structural input. At the very least, I'd prefer not to go into that direction for this change.

[Hahnfeld]

ping

[v.g.vassilev]

This looks reasonable to me, but I'd wait for @aaron.ballman greenlight as he seems to have had more comments/ideas.

clang/lib/Lex/Preprocessor.cpp

1003

I am not sure I understand the reasoning but I somewhat see that having Parser's SkipUntil be implemented with our new Preprocessor::LexUntil... would require a lot more work. How about adding a fixme note capturing this as a future possible refactoring?

[aaron.ballman]

LGTM aside from a minor naming nit. I think finding a way to unify with SkipUntil would be good follow-up work, though.

clang/lib/Lex/Preprocessor.cpp

1003

Minor naming convention nit.

[Hahnfeld]

Address minor naming convention nit.

clang/lib/Lex/Preprocessor.cpp

1003

@v.g.vassilev Parser::SkipUntil has a long switch statement looking at the token and taking special actions depending on their kind and the context it appears in. I don't see how to generalize this in the way LexTokensUntilEOF works.

clang/include/clang/Lex/Preprocessor.h

public:
 
   /// True if EnableBacktrackAtThisPos() was called and
   /// caching of tokens is on.
   bool isBacktrackEnabled() const { return !BacktrackPositions.empty(); }
 
   /// Lex the next token for this preprocessor.
   void Lex(Token &Result);
 
+  /// Lex all tokens for this preprocessor until (and excluding) end of file.
+  void LexTokensUntilEOF(std::vector<Token> *Tokens = nullptr);
+
   /// Lex a token, forming a header-name token if possible.
   bool LexHeaderName(Token &Result, bool AllowMacroExpansion = true);
 
   bool LexAfterModuleImport(Token &Result);
   void CollectPpImportSuffix(SmallVectorImpl<Token> &Toks);
 
   void makeModuleVisible(Module *M, SourceLocation Loc);
 

clang/lib/Lex/Preprocessor.cpp

if ((LexLevel == 0 || PreprocessToken) &&
       !Result.getFlag(Token::IsReinjected)) {
     if (LexLevel == 0)
       ++TokenCount;
     if (OnToken)
       OnToken(Result);
   }
 }
 
+void Preprocessor::LexTokensUntilEOF(std::vector<Token> *Tokens) {
+  while (1) {
+    Token Tok;
+    Lex(Tok);
+    if (Tok.isOneOf(tok::unknown, tok::eof, tok::eod))
+      break;
+    if (Tokens != nullptr)
+      Tokens->push_back(Tok);
+  }
+}
+
 /// Lex a header-name token (including one formed from header-name-tokens if
 /// \p AllowConcatenation is \c true).
 ///
 /// \param FilenameTok Filled in with the next token. On success, this will
 ///        be either a header_name token. On failure, it will be whatever other
 ///        token was found instead.
 /// \param AllowMacroExpansion If \c true, allow the header name to be formed
 ///        by macro expansion (concatenating tokens as necessary if the first

clang/unittests/Analysis/MacroExpansionContextTest.cpp

getMacroExpansionContextFor(StringRef SourceText) {
 
     PP.Initialize(*Target);
     auto Ctx = std::make_unique<MacroExpansionContext>(LangOpts);
     Ctx->registerForPreprocessor(PP);
 
     // Lex source text.
     PP.EnterMainSourceFile();
 
-    while (true) {
-      Token Tok;
-      PP.Lex(Tok);
-      if (Tok.is(tok::eof))
-        break;
-    }
+    PP.LexTokensUntilEOF();
 
     // Callbacks have been executed at this point.
     return Ctx;
   }
 
   /// Returns the expansion location to main file at the given row and column.
   SourceLocation at(unsigned row, unsigned col) const {
     SourceLocation Loc =

clang/unittests/Basic/SourceManagerTest.cpp

TEST_F(SourceManagerTest, isBeforeInTranslationUnit) {
   Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
                   SourceMgr, HeaderInfo, ModLoader,
                   /*IILookup =*/nullptr,
                   /*OwnsHeaderSearch =*/false);
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 
   std::vector<Token> toks;
-  while (1) {
-    Token tok;
-    PP.Lex(tok);
-    if (tok.is(tok::eof))
-      break;
-    toks.push_back(tok);
-  }
+  PP.LexTokensUntilEOF(&toks);
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(3U, toks.size());
   ASSERT_EQ(tok::l_square, toks[0].getKind());
   ASSERT_EQ(tok::identifier, toks[1].getKind());
   ASSERT_EQ(tok::r_square, toks[2].getKind());
   
   SourceLocation lsqrLoc = toks[0].getLocation();
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
                   SourceMgr, HeaderInfo, ModLoader,
                   /*IILookup =*/nullptr,
                   /*OwnsHeaderSearch =*/false);
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
   llvm::SmallString<8> Scratch;
 
   std::vector<Token> toks;
-  while (1) {
-    Token tok;
-    PP.Lex(tok);
-    if (tok.is(tok::eof))
-      break;
-    toks.push_back(tok);
-  }
+  PP.LexTokensUntilEOF(&toks);
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(4U, toks.size()) << "a >> b c";
   // Sanity check their order.
   for (unsigned I = 0; I < toks.size() - 1; ++I) {
     EXPECT_TRUE(SourceMgr.isBeforeInTranslationUnit(toks[I].getLocation(),
                                                     toks[I + 1].getLocation()));
     EXPECT_FALSE(SourceMgr.isBeforeInTranslationUnit(toks[I + 1].getLocation(),
TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
   // These are different than normal includes since predefines buffer doesn't
   // have a valid insertion location.
   PP.setPredefines("#include \"/implicit-header.h\"");
   FileMgr.getVirtualFile("/implicit-header.h", 0, 0);
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 
   std::vector<Token> toks;
-  while (1) {
-    Token tok;
-    PP.Lex(tok);
-    if (tok.is(tok::eof))
-      break;
-    toks.push_back(tok);
-  }
+  PP.LexTokensUntilEOF(&toks);
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(4U, toks.size());
   ASSERT_EQ(tok::numeric_constant, toks[0].getKind());
   ASSERT_EQ(tok::numeric_constant, toks[1].getKind());
   ASSERT_EQ(tok::numeric_constant, toks[2].getKind());
   ASSERT_EQ(tok::numeric_constant, toks[3].getKind());
 
TEST_F(SourceManagerTest, isBeforeInTranslationUnitWithMacroInInclude) {
   PP.Initialize(*Target);
 
   std::vector<MacroAction> Macros;
   PP.addPPCallbacks(std::make_unique<MacroTracker>(Macros));
 
   PP.EnterMainSourceFile();
 
   std::vector<Token> toks;
-  while (1) {
-    Token tok;
-    PP.Lex(tok);
-    if (tok.is(tok::eof))
-      break;
-    toks.push_back(tok);
-  }
+  PP.LexTokensUntilEOF(&toks);
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(0U, toks.size());
 
   ASSERT_EQ(15U, Macros.size());
   // #define M(x) x
   ASSERT_TRUE(Macros[0].isDefinition());
   ASSERT_EQ("M", Macros[0].Name);

clang/unittests/Lex/LexerTest.cpp

std::unique_ptr<Preprocessor> CreatePP(StringRef Source,
     return PP;
   }
 
   std::vector<Token> Lex(StringRef Source) {
     TrivialModuleLoader ModLoader;
     PP = CreatePP(Source, ModLoader);
 
     std::vector<Token> toks;
-    while (1) {
-      Token tok;
-      PP->Lex(tok);
-      if (tok.is(tok::eof))
-        break;
-      toks.push_back(tok);
-    }
+    PP->LexTokensUntilEOF(&toks);
 
     return toks;
   }
 
   std::vector<Token> CheckLex(StringRef Source,
                               ArrayRef<tok::TokenKind> ExpectedTokens) {
     auto toks = Lex(Source);
     EXPECT_EQ(ExpectedTokens.size(), toks.size());
TEST_F(LexerTest, FindNextToken) {
   }
   EXPECT_THAT(GeneratedByNextToken, ElementsAre("abcd", "=", "0", ";", "int",
                                                 "xyz", "=", "abcd", ";"));
 }
 
 TEST_F(LexerTest, CreatedFIDCountForPredefinedBuffer) {
   TrivialModuleLoader ModLoader;
   auto PP = CreatePP("", ModLoader);
-  while (1) {
-    Token tok;
-    PP->Lex(tok);
-    if (tok.is(tok::eof))
-      break;
-  }
+  PP->LexTokensUntilEOF();
   EXPECT_EQ(SourceMgr.getNumCreatedFIDsForFileID(PP->getPredefinesFileID()),
             1U);
 }
 
 TEST_F(LexerTest, RawAndNormalLexSameForLineComments) {
   const llvm::StringLiteral Source = R"cpp(
   // First line comment.
   //* Second line comment which is ambigious.

clang/unittests/Lex/ModuleDeclStateTest.cpp

return std::make_unique<Preprocessor>(
         /*OwnsHeaderSearch =*/false);
   }
 
   void preprocess(Preprocessor &PP, std::unique_ptr<PPCallbacks> C) {
     PP.Initialize(*Target);
     PP.addPPCallbacks(std::move(C));
     PP.EnterMainSourceFile();
 
-    while (1) {
-      Token tok;
-      PP.Lex(tok);
-      if (tok.is(tok::eof))
-        break;
-    }
+    PP.LexTokensUntilEOF();
   }
 
   FileSystemOptions FileMgrOpts;
   FileManager FileMgr;
   IntrusiveRefCntPtr<DiagnosticIDs> DiagID;
   DiagnosticsEngine Diags;
   SourceManager SourceMgr;
   std::shared_ptr<TargetOptions> TargetOpts;

clang/unittests/Lex/PPCallbacksTest.cpp

protected:
 
   InclusionDirectiveCallbacks *InclusionDirectiveCallback(Preprocessor &PP) {
     PP.Initialize(*Target);
     InclusionDirectiveCallbacks* Callbacks = new InclusionDirectiveCallbacks;
     PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));
 
     // Lex source text.
     PP.EnterMainSourceFile();
-
-    while (true) {
-      Token Tok;
-      PP.Lex(Tok);
-      if (Tok.is(tok::eof))
-        break;
-    }
+    PP.LexTokensUntilEOF();
 
     // Callbacks have been executed at this point -- return filename range.
     return Callbacks;
   }
 
   std::vector<CondDirectiveCallbacks::Result>
   DirectiveExprRange(StringRef SourceText) {
     TrivialModuleLoader ModLoader;
     std::unique_ptr<llvm::MemoryBuffer> Buf =
         llvm::MemoryBuffer::getMemBuffer(SourceText);
     SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
     HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                             Diags, LangOpts, Target.get());
     Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
                     SourceMgr, HeaderInfo, ModLoader,
                     /*IILookup =*/nullptr,
                     /*OwnsHeaderSearch =*/false);
     PP.Initialize(*Target);
     auto *Callbacks = new CondDirectiveCallbacks;
     PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));
 
     // Lex source text.
     PP.EnterMainSourceFile();
-
-    while (true) {
-      Token Tok;
-      PP.Lex(Tok);
-      if (Tok.is(tok::eof))
-        break;
-    }
+    PP.LexTokensUntilEOF();
 
     return Callbacks->Results;
   }
 
   std::vector<PragmaMarkCallbacks::Mark>
   PragmaMarkCall(const char *SourceText) {
     std::unique_ptr<llvm::MemoryBuffer> SourceBuf =
         llvm::MemoryBuffer::getMemBuffer(SourceText, "test.c");
     SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(SourceBuf)));
 
     HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                             Diags, LangOpts, Target.get());
     TrivialModuleLoader ModLoader;
 
     Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
                     SourceMgr, HeaderInfo, ModLoader, /*IILookup=*/nullptr,
                     /*OwnsHeaderSearch=*/false);
     PP.Initialize(*Target);
 
     auto *Callbacks = new PragmaMarkCallbacks;
     PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));
 
     // Lex source text.
     PP.EnterMainSourceFile();
-    while (true) {
-      Token Tok;
-      PP.Lex(Tok);
-      if (Tok.is(tok::eof))
-        break;
-    }
+    PP.LexTokensUntilEOF();
 
     return Callbacks->Marks;
   }
 
   PragmaOpenCLExtensionCallbacks::CallbackParameters
   PragmaOpenCLExtensionCall(const char *SourceText) {
     LangOptions OpenCLLangOpts;
     OpenCLLangOpts.OpenCL = 1;
PragmaOpenCLExtensionCall(const char *SourceText) {
     ASTConsumer Consumer;
     Sema S(PP, Context, Consumer);
     Parser P(PP, S, false);
     PragmaOpenCLExtensionCallbacks* Callbacks = new PragmaOpenCLExtensionCallbacks;
     PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));
 
     // Lex source text.
     PP.EnterMainSourceFile();
-    while (true) {
-      Token Tok;
-      PP.Lex(Tok);
-      if (Tok.is(tok::eof))
-        break;
-    }
+    PP.LexTokensUntilEOF();
 
     PragmaOpenCLExtensionCallbacks::CallbackParameters RetVal = {
       Callbacks->Name,
       Callbacks->State
     };
     return RetVal;
   }
 };
public:
     }
   };
 
   auto *Callbacks = new FileNotFoundCallbacks;
   PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));
 
   // Lex source text.
   PP.EnterMainSourceFile();
-  while (true) {
-    Token Tok;
-    PP.Lex(Tok);
-    if (Tok.is(tok::eof))
-      break;
-  }
+  PP.LexTokensUntilEOF();
 
   ASSERT_EQ(1u, Callbacks->NumCalls);
   ASSERT_EQ(0u, DiagConsumer->getNumErrors());
 }
 
 TEST_F(PPCallbacksTest, OpenCLExtensionPragmaEnabled) {
   const char* Source =
     "#pragma OPENCL EXTENSION cl_khr_fp64 : enable\n";

clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp

Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
                   /*OwnsHeaderSearch =*/false);
   PP.Initialize(*Target);
   PPConditionalDirectiveRecord *
     PPRec = new PPConditionalDirectiveRecord(SourceMgr);
   PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(PPRec));
   PP.EnterMainSourceFile();
 
   std::vector<Token> toks;
-  while (1) {
-    Token tok;
-    PP.Lex(tok);
-    if (tok.is(tok::eof))
-      break;
-    toks.push_back(tok);
-  }
+  PP.LexTokensUntilEOF(&toks);
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(10U, toks.size());
   
   EXPECT_FALSE(PPRec->rangeIntersectsConditionalDirective(
                     SourceRange(toks[0].getLocation(), toks[1].getLocation())));
   EXPECT_TRUE(PPRec->rangeIntersectsConditionalDirective(
                     SourceRange(toks[0].getLocation(), toks[2].getLocation())));

clang/unittests/Lex/PPDependencyDirectivesTest.cpp

TEST_F(PPDependencyDirectivesTest, MacroGuard) {
   Preprocessor PP(PPOpts, Diags, LangOpts, SourceMgr, HeaderInfo, ModLoader,
                   /*IILookup =*/nullptr,
                   /*OwnsHeaderSearch =*/false);
   PP.Initialize(*Target);
 
   SmallVector<StringRef> IncludedFiles;
   PP.addPPCallbacks(std::make_unique<IncludeCollector>(PP, IncludedFiles));
   PP.EnterMainSourceFile();
-  while (true) {
-    Token tok;
-    PP.Lex(tok);
-    if (tok.is(tok::eof))
-      break;
-  }
+  PP.LexTokensUntilEOF();
 
   SmallVector<std::string> IncludedFilesSlash;
   for (StringRef IncludedFile : IncludedFiles)
     IncludedFilesSlash.push_back(
         llvm::sys::path::convert_to_slash(IncludedFile));
   SmallVector<std::string> ExpectedIncludes{
       "main.c", "./head1.h", "./head2.h", "./head2.h", "./head3.h", "./head3.h",
   };
   EXPECT_EQ(IncludedFilesSlash, ExpectedIncludes);
 }
 
 } // anonymous namespace

clang/unittests/Lex/PPMemoryAllocationsTest.cpp

HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                           Diags, LangOpts, Target.get());
   Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
                   SourceMgr, HeaderInfo, ModLoader,
                   /*IILookup =*/nullptr,
                   /*OwnsHeaderSearch =*/false);
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 
-  while (1) {
-    Token tok;
-    PP.Lex(tok);
-    if (tok.is(tok::eof))
-      break;
-  }
+  PP.LexTokensUntilEOF();
 
   size_t NumAllocated = PP.getPreprocessorAllocator().getBytesAllocated();
   float BytesPerDefine = float(NumAllocated) / float(NumMacros);
   llvm::errs() << "Num preprocessor allocations for " << NumMacros
                << " #define: " << NumAllocated << "\n";
   llvm::errs() << "Bytes per #define: " << BytesPerDefine << "\n";
   // On arm64-apple-macos, we get around 120 bytes per define.
   // Assume a reasonable upper bound based on that number that we don't want
   // to exceed when storing information about a macro #define with 1 or 3
   // tokens.
   EXPECT_LT(BytesPerDefine, 130.0f);
 }
 
 } // anonymous namespace