This is an archive of the discontinued LLVM Phabricator instance.

[clangd] NFC, emit source ranges in selection debug messages.
ClosedPublic

Authored by hokein on Jan 17 2022, 6:20 AM.

Details

Summary

It will make the output more versbose, but I found that these are useful
information when debugging selection tree.

Diff Detail

Event Timeline

hokein created this revision.Jan 17 2022, 6:20 AM
hokein requested review of this revision.Jan 17 2022, 6:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 17 2022, 6:20 AM
sammccall added inline comments.Jan 18 2022, 2:29 PM
clang-tools-extra/clangd/Selection.cpp
550–551

I don't think this belongs in printNodeToString but rather explicitly in the log statements that call it.

780–781

We're already printing the range here, which only differs rarely. This seems noisy and confusing.
(S is a better range to print than N.getSourceRange here - no objection if you want to change the log formatting to print it similarly to the others)

814

are we sure we want to print the range again on pop? Seems like on enter/skip is sufficient. It's useful to be able to visually distinguish push vs pop.

hokein updated this revision to Diff 401219.Jan 19 2022, 6:39 AM

address commment, only printing range on push.

clang-tools-extra/clangd/Selection.cpp
780–781

I inlined these two into one dlog, which makes the output more dense.

814

oh, right, we don't need that. I think printing range on push is sufficient.

sammccall accepted this revision.Jan 19 2022, 6:49 AM

Thanks!

clang-tools-extra/clangd/Selection.cpp
550–551

hmm, while here, this extra space seems weird.

Previously it was at EOL and invisible.

After this patch it's used, but I think it'd be clearer to move it to the format string.

This revision is now accepted and ready to land.Jan 19 2022, 6:49 AM
This revision was landed with ongoing or failed builds.Jan 19 2022, 7:07 AM
This revision was automatically updated to reflect the committed changes.