Skip to content

Commit

Permalink
[clang] Make sure argument expansion locations are correct in presenc…
Browse files Browse the repository at this point in the history
…e of predefined buffer

Summary:
Macro argument expansion logic relies on skipping file IDs that created
as a result of an include. Unfortunately it fails to do that for
predefined buffer since it doesn't have a valid insertion location.

As a result of that any file ID created for an include inside the
predefined buffers breaks the traversal logic in
SourceManager::computeMacroArgsCache.

To fix this issue we first record number of created FIDs for predefined
buffer, and then skip them explicitly in source manager.

Another solution would be to just give predefined buffers a valid source
location, but it is unclear where that should be..

Reviewers: sammccall

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78649
  • Loading branch information
kadircet committed Apr 22, 2020
1 parent 7d1ee63 commit 411a254
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 9 deletions.
24 changes: 16 additions & 8 deletions clang/lib/Basic/SourceManager.cpp
Expand Up @@ -1800,15 +1800,23 @@ void SourceManager::computeMacroArgsCache(MacroArgsMap &MacroArgsCache,
return;
if (Entry.isFile()) {
SourceLocation IncludeLoc = Entry.getFile().getIncludeLoc();
if (IncludeLoc.isInvalid())
bool IncludedInFID =
(IncludeLoc.isValid() && isInFileID(IncludeLoc, FID)) ||
// Predefined header doesn't have a valid include location in main
// file, but any files created by it should still be skipped when
// computing macro args expanded in the main file.
(FID == MainFileID && Entry.getFile().Filename == "<built-in>");
if (IncludedInFID) {
// Skip the files/macros of the #include'd file, we only care about
// macros that lexed macro arguments from our file.
if (Entry.getFile().NumCreatedFIDs)
ID += Entry.getFile().NumCreatedFIDs - 1 /*because of next ++ID*/;
continue;
if (!isInFileID(IncludeLoc, FID))
return; // No more files/macros that may be "contained" in this file.

// Skip the files/macros of the #include'd file, we only care about macros
// that lexed macro arguments from our file.
if (Entry.getFile().NumCreatedFIDs)
ID += Entry.getFile().NumCreatedFIDs - 1/*because of next ++ID*/;
} else if (IncludeLoc.isValid()) {
// If file was included but not from FID, there is no more files/macros
// that may be "contained" in this file.
return;
}
continue;
}

Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Lex/PPLexerChange.cpp
Expand Up @@ -415,7 +415,10 @@ bool Preprocessor::HandleEndOfFile(Token &Result, bool isEndOfMacro) {
}

if (!isEndOfMacro && CurPPLexer &&
SourceMgr.getIncludeLoc(CurPPLexer->getFileID()).isValid()) {
(SourceMgr.getIncludeLoc(CurPPLexer->getFileID()).isValid() ||
// Predefines file doesn't have a valid include location.
(PredefinesFileID.isValid() &&
CurPPLexer->getFileID() == PredefinesFileID))) {
// Notify SourceManager to record the number of FileIDs that were created
// during lexing of the #include'd file.
unsigned NumFIDs =
Expand Down
6 changes: 6 additions & 0 deletions clang/unittests/Basic/SourceManagerTest.cpp
Expand Up @@ -294,10 +294,16 @@ TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
TrivialModuleLoader ModLoader;
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
Diags, LangOpts, &*Target);

Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
SourceMgr, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
// Ensure we can get expanded locations in presence of implicit includes.
// 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();

Expand Down
13 changes: 13 additions & 0 deletions clang/unittests/Lex/LexerTest.cpp
Expand Up @@ -556,4 +556,17 @@ 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;
}
EXPECT_EQ(SourceMgr.getNumCreatedFIDsForFileID(PP->getPredefinesFileID()),
1U);
}
} // anonymous namespace

0 comments on commit 411a254

Please sign in to comment.