Page MenuHomePhabricator

[clangd] Add ClangdServer::customAction() extension point
ClosedPublic

Authored by sammccall on Aug 13 2020, 1:44 AM.

Details

Summary

This lets basic AST-consuming actions be defined outside clangdserver.
(it essentially exposes TUScheduler::runWithAST).

Two use cases for now:

  • replace ClangdServer::dumpAST, which doesn't really belong in the public interface (a followup patch will do this)
  • allow embedders to add/experiment with extra features (e.g. I know some who want to try crazy AST+ML code completion...)

Maybe in the future we should use this sort of mechanism to make ClangdServer
less of a monolith, but that's not in scope for now.
This would probably require a richer and more carefully-thought-out API.

Diff Detail

Event Timeline

sammccall created this revision.Aug 13 2020, 1:44 AM
sammccall requested review of this revision.Aug 13 2020, 1:44 AM

mostly looks good to me, as discussed offline I would rather expose AST within a thread, both to keep the API changes to a minimum in the future and possibly indicating the "view-ness" of the exposed structs like AST more explicitly.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 13 2020, 3:26 AM
This revision was automatically updated to reflect the committed changes.

Oops sorry, misread "mostly lgtm" and committed.

Changed to use the "InputsAndAST" struct to provide the rest of the info.
Happy to address any further comments...

no I am totally fine with the change. I just wanted to give Adam some time to take a look too :D

Yup sorry @adamcz, I was just confused. Likewise, happy to change to a different structure or revert entirely if you have concerns.

41d0edd54e29e994fa7d40961a38e8fca27addac eliminates dumpAST based on this, but is easy to revert too.