[PATCH] D119708: [clang][lex] Remove `PPCallbacks::FileNotFound()`

Tue Jan 17 23:29:16 PST 2023

Hahnfeld added a comment.

In D119708#4060336 <https://reviews.llvm.org/D119708#4060336>, @jansvoboda11 wrote:

> In D119708#4059254 <https://reviews.llvm.org/D119708#4059254>, @Hahnfeld wrote:
>
>> Hello, sorry for the late heads-up, but this functionality is used by ROOT: https://github.com/root-project/root/blob/f58cccf5ce7fd67894c7fd9e9e74d3f37bc1acba/core/metacling/src/TClingCallbacks.cxx#L282 Any chance of bringing this back?
>
> Hi. Your downstream callback never fills in `RecoveryPath` and always returns `false`, meaning it will never reach the unsafe block of code I had issue with: `HeaderInfo.AddSearchPath(DL, isAngled)`.

This is true, but it provides us a hook to do something in case a file is not found. For ROOT, we "extend" the `#include` syntax and allow "modifiers" at the end. Then we do something special (completely on our own) for things like `#include "myfile.C+"` (note the plus).

> Technically, we could bring back a safe/trimmed-down version of this callback that doesn't take the out-parameter and returns `void`. But I don't think it's the right call, since that would essentially re-introduce dead code to upstream.
> This is probably best kept downstream, unless there are other downstream users.

Sure, but how would we do that without a hook? At the moment, we try very hard to reduce the number of patches that we carry for Clang (which is also why parts of Cling are upstreamed into `clang-repl`).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119708/new/

https://reviews.llvm.org/D119708