[PATCH] D114525: [clang] Change ordering of PreableCallbacks to make sure PP can be referenced in them

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 24 05:07:22 PST 2021


kadircet added a comment.

As discussed offline this definitely LGTM, let me summarize the reasoning here.

The original review was in https://reviews.llvm.org/D41365. It's unclear why they went with before `BeginSourceFile`. In theory having the callback issued before `BeginSourceFile` allows setting various options that'll effect parsing, but today there's only one user (at least in the upstream) of this callback, clangd, which uses the callack to stash some state from CompilerIntance rather than mutating it.
If we ever have users that'll need to mutate the CompilerIntance before `BeginSourceFile`, I think we should have a particular callback for that use case then.

Will still wait for feedback from @sammccall as he might have some historical context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114525



More information about the cfe-commits mailing list