Page MenuHomePhabricator

[clangd] Add hidden tweaks to dump AST/selection.
ClosedPublic

Authored by sammccall on May 28 2019, 11:02 AM.

Details

Summary

This introduces a few new concepts:

  • tweaks have an Intent (they don't all advertise as refactorings)
  • tweaks may produce messages (for ShowMessage notification). Generalized Replacements -> Effect.
  • tweaks (and other features) may be hidden (clangd -hidden-features flag). We may choose to promote these one day. I'm not sure they're worth their own feature flags though.

Verified it in vim-clangd (not yet open source), curious if the UI is ok in VSCode.

Diff Detail

Repository
rL LLVM

Event Timeline

sammccall created this revision.May 28 2019, 11:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 28 2019, 11:02 AM
sammccall updated this revision to Diff 201765.May 28 2019, 2:31 PM

Add DumpRecordLayout. Fix a small SelectionTree bug uncovered by these tweaks.

ilya-biryukov accepted this revision.Jun 18 2019, 1:54 AM

LGTM with a few NITs

clangd/refactor/Tweak.h
104 ↗(On Diff #201765)

I wonder whether this should be a static method. WDYT?

That would allow to even prevent calling prepare() on those tweaks.
OTOH, we want prepare() should be fast and it shouldn't matter if that's the case.

clangd/unittests/TweakTests.cpp
19 ↗(On Diff #201765)

NIT: the include is redundant. Maybe remove? (probably added by clangd)

This revision is now accepted and ready to land.Jun 18 2019, 1:54 AM
sammccall marked 2 inline comments as done.Jun 18 2019, 4:58 AM
sammccall added inline comments.
clangd/refactor/Tweak.h
104 ↗(On Diff #201765)

We'd need to add criteria to prepareTweaks() as hidden() couldn't be checked polymorphically on the results.

I think this is a good idea but should probably happen if/when we have a more important reason to touch that API.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 18 2019, 6:36 AM