This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Enable folding ranges by default.
ClosedPublic

Authored by usaxena95 on Aug 30 2022, 2:08 AM.

Diff Detail

Event Timeline

usaxena95 created this revision.Aug 30 2022, 2:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 30 2022, 2:08 AM
Herald added a subscriber: arphaman. · View Herald Transcript
usaxena95 requested review of this revision.Aug 30 2022, 2:08 AM

Whenever I tried this option in the past it crashed clangd.
I recommend doing some more testing before flipping the switch.

kadircet added inline comments.Aug 30 2022, 2:26 AM
clang-tools-extra/clangd/tool/ClangdMain.cpp
337–338

i think we should just retire the flag altogether, ATM this is only preventing clangdlspserver from announcing the capability.

the only benefit of keeping it as a flag is, we can turn it off quickly if its crashing constantly. but we should fix those crashes instead. so do you mind:

  • moving it to be near other retiredflags (around line 320)
  • always announce foldingrangeprovider capability and bind the method in clangdlspserver.cpp
  • get rid of the option inside clangdserver.h

Whenever I tried this option in the past it crashed clangd.
I recommend doing some more testing before flipping the switch.

we've a new implementation of folding ranges based on pseudo parsing now. hence no more crashes 🎉

usaxena95 updated this revision to Diff 456584.Aug 30 2022, 2:50 AM
usaxena95 marked an inline comment as done.

Addressed comments.

kadircet accepted this revision.Aug 30 2022, 2:59 AM

thanks, lgtm!

This revision is now accepted and ready to land.Aug 30 2022, 2:59 AM
This revision was landed with ongoing or failed builds.Aug 30 2022, 3:05 AM
This revision was automatically updated to reflect the committed changes.