Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions clang/include/clang/Lex/PPCallbacks.h
Original file line number Diff line number Diff line change
Expand Up @@ -499,10 +499,10 @@ class PPChainedCallbacks : public PPCallbacks {
}

bool EmbedFileNotFound(StringRef FileName) override {
bool Skip = First->FileNotFound(FileName);
bool Skip = First->EmbedFileNotFound(FileName);
// Make sure to invoke the second callback, no matter if the first already
// returned true to skip the file.
Skip |= Second->FileNotFound(FileName);
Skip |= Second->EmbedFileNotFound(FileName);
return Skip;
}

Expand Down
11 changes: 6 additions & 5 deletions clang/lib/Lex/PPDirectives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1397,11 +1397,12 @@ void Preprocessor::HandleDirective(Token &Result) {
return HandleIdentSCCSDirective(Result);
case tok::pp_sccs:
return HandleIdentSCCSDirective(Result);
case tok::pp_embed:
return HandleEmbedDirective(SavedHash.getLocation(), Result,
getCurrentFileLexer()
? *getCurrentFileLexer()->getFileEntry()
: static_cast<FileEntry *>(nullptr));
case tok::pp_embed: {
if (auto *CurrentFileLexer = getCurrentFileLexer())
if (auto FERef = CurrentFileLexer->getFileEntry())
return HandleEmbedDirective(SavedHash.getLocation(), Result, *FERef);
return HandleEmbedDirective(SavedHash.getLocation(), Result, nullptr);
}
case tok::pp_assert:
//isExtension = true; // FIXME: implement #assert
break;
Expand Down
34 changes: 34 additions & 0 deletions clang/unittests/Lex/PPCallbacksTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,40 @@ TEST_F(PPCallbacksTest, FileNotFoundSkipped) {
ASSERT_EQ(0u, DiagConsumer->getNumErrors());
}

TEST_F(PPCallbacksTest, EmbedFileNotFoundChained) {
const char *SourceText = "#embed \"notfound.h\"\n";

std::unique_ptr<llvm::MemoryBuffer> SourceBuf =
llvm::MemoryBuffer::getMemBuffer(SourceText);
SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(SourceBuf)));

HeaderSearchOptions HSOpts;
TrivialModuleLoader ModLoader;
PreprocessorOptions PPOpts;
HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, Target.get());

DiagnosticConsumer *DiagConsumer = new DiagnosticConsumer;
DiagnosticsEngine EmbedFileNotFoundDiags(DiagID, DiagOpts, DiagConsumer);
Preprocessor PP(PPOpts, EmbedFileNotFoundDiags, LangOpts, SourceMgr,
HeaderInfo, ModLoader, /*IILookup=*/nullptr,
/*OwnsHeaderSearch=*/false);
PP.Initialize(*Target);

class EmbedFileNotFoundCallbacks : public PPCallbacks {
public:
bool EmbedFileNotFound(StringRef FileName) override { return true; }
};

PP.addPPCallbacks(std::make_unique<EmbedFileNotFoundCallbacks>());
PP.addPPCallbacks(std::make_unique<EmbedFileNotFoundCallbacks>());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what is the point of adding the two of these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said, the bug is only triggered when we have a PPChainedCallbacks. That is, if you only add one, the test passes, if you add two, the test fails before the patch. Again, I don't think this is a situation where a test case is needed, but if one is needed, it must look like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a comment explaining why there are two


// Lex source text.
PP.EnterMainSourceFile();
PP.LexTokensUntilEOF();

ASSERT_EQ(0u, DiagConsumer->getNumErrors());
}

TEST_F(PPCallbacksTest, OpenCLExtensionPragmaEnabled) {
const char* Source =
"#pragma OPENCL EXTENSION cl_khr_fp64 : enable\n";
Expand Down