[PATCH] D92290: [clangd] Use clangd's Context mechanism to make the ASTContext of the AST being operated on available everywhere

Fri Dec 18 00:33:04 PST 2020

nridge added a comment.

In D92290#2460748 <https://reviews.llvm.org/D92290#2460748>, @sammccall wrote:

> my manager was appalled when we added it...

:-)

> Does this enable some future work, or is it reasonable to evaluate this patch in isolation?

I was inspired to do this by a work-in-progress patch I have to fix https://github.com/clangd/clangd/issues/451, for which I need to make `Sema` available in a similar way. That patch isn't quite ready for review (there are some crashes / assertion failures that I need to iron out), but I posted what I have so far at D93522 <https://reviews.llvm.org/D93522> if you're curious.

> Here it's a mandatory input, and the alternative is cluttering a couple of layers of purely-private interfaces, so this seems like a lot of magic to avoid a fairly small amount of awkwardness.

I agree that there is no fundamental need to use `Context`, it just seemed convenient (and hey, `ASTContext` even has `Context` in its name :-)). I could just pass the `ASTContext` as a parameter to `targetDecl()` and other relevant functions.

Perhaps, with a view to eventually needing `Sema` as well, I should just pass `ParsedAST` instead?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92290