This is an archive of the discontinued LLVM Phabricator instance.

[Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback
ClosedPublic

Authored by akyrtzi on Jun 30 2022, 2:29 PM.

Details

Summary

This is a preprocessor callback focused on the lexed file changing, without conflating effects of line number directives and other pragmas.
A client that only cares about what files the lexer processes, like dependency generation, can use this more straightforward
callback instead of PPCallbacks::FileChanged(). Clients that want the pragma directive effects as well can keep using FileChanged().

A use case where PPCallbacks::LexedFileChanged() is particularly simpler to use than FileChanged() is in a situation
where a client wants to keep track of lexed file changes that include changes from/to the predefines buffer, where it becomes
unnecessary complicated trying to use FileChanged() while filtering out the pragma directives effects callbacks.

For LexedFileChanged() take the opportunity to also provide information about the prior FileID the Lexer moved from, even when
entering a new file.

Diff Detail

Event Timeline

akyrtzi created this revision.Jun 30 2022, 2:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 2:29 PM
akyrtzi requested review of this revision.Jun 30 2022, 2:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 2:29 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
benlangmuir added inline comments.Jun 30 2022, 3:37 PM
clang/lib/Basic/SourceManager.cpp
1014–1020

Just a suggestion: value_or can be a nice way to express simple cases like this:

getFilename(getFileID(SpellingLoc)).value_or(StringRef());
clang/lib/Lex/PPLexerChange.cpp
135

Why does LexedFileChanged have PrevFID set, but FileChanged does not (it has a default argument of FileID()? I would have expected that when you call both FileChanged and LexedFileChanged for the same event this would match.

akyrtzi added inline comments.Jun 30 2022, 4:32 PM
clang/lib/Basic/SourceManager.cpp
1014–1020

Thank you for the tip!

clang/lib/Lex/PPLexerChange.cpp
135

I didn't want to change the "contract" of FileChanged() as part of these changes but it's probably unlikely that someone depends on FileID being invalid, I'll give it a try.

akyrtzi updated this revision to Diff 441551.Jun 30 2022, 4:44 PM

Use Optional::value_or()

akyrtzi updated this revision to Diff 441556.Jun 30 2022, 4:56 PM

Pass a value for PrevFID for FileChanged() callback as well, for PPCallbacks::EnterFile reason.

akyrtzi updated this revision to Diff 441587.Jun 30 2022, 10:36 PM

Update clang-tools-extra/test/pp-trace/pp-trace-include.cpp to accomodate for PrevFID getting a value and
preserve using getFileEntryForID() for the SourceManager::getFilename() implementation.

Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 10:36 PM
akyrtzi updated this revision to Diff 441700.Jul 1 2022, 8:25 AM

Avoid changing the SourceManager::getFilename() implementation.

akyrtzi added inline comments.Jul 1 2022, 10:01 AM
clang/lib/Lex/PPLexerChange.cpp
135

I'm passing a PrevFID value for FileChanged() as well and I only had to update one test.

benlangmuir accepted this revision.Jul 1 2022, 2:18 PM
This revision is now accepted and ready to land.Jul 1 2022, 2:18 PM