This callback was removed with commit rG7a124f4859, but we use it downstream in ROOT/Cling to implement handling of a special include syntax. Add back a "safe" version of the callback that only takes the file name and return a bool to silently skip the file.
Details
Diff Detail
Unit Tests
Event Timeline
I just checked and this "simplified" version of the callback would give us enough information to implement all we need in ROOT (plus some, the bool return value is nicer than temporarily playing with SuppressIncludeNotFoundError). For an upstream test, I could implement a unit test in clang/unittests/Lex/PPCallbacksTest.cpp or a "toy" tooling where #include <file.h?> is ignored if file.h doesn't exist? What do you think (assuming the general idea of adding this callback is fine)?
LGTM with improved wording.
clang/include/clang/Lex/PPCallbacks.h | ||
---|---|---|
87 | The wording is a bit misleading due to: if (SuppressIncludeNotFoundError) return std::nullopt; after invoking this callback. Reading the comment, I assume the diagnostic has already been issued. Can we tweak this to clarify things? |
Also I think we should mention this API in our docs, right where the removed original used to be.
Which API docs are you referring to? I'm only aware of the pp-trace docs (updated in https://reviews.llvm.org/D125258 for the removal), but I didn't add FileNotFound to pp-trace because it's more of a hook than a "traceable event" - should I add it nevertheless?
clang/include/clang/Lex/PPCallbacks.h | ||
---|---|---|
87 | Please let me know if this sounds better. |
I checked a second time and couldn't find other API docs. I've now pushed this change for now (also to get it included in release/16.x), but please let me know for any follow-ups. Thanks!
Yes, I meant the pp-trace docs. Why would "we didn't find the included file" not be a traceable event?
My thinking is the following: If the preprocessor cannot find the included file and the Hook doesn't return true, it will run into a fatal error and stop parsing the source file. I don't know if that's actually a relevant case, but I have no experience with pp-trace, so maybe?
The wording is a bit misleading due to:
after invoking this callback. Reading the comment, I assume the diagnostic has already been issued. Can we tweak this to clarify things?