This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Bring back early-claim approach to fix a selection-tree regression.
ClosedPublic

Authored by hokein on Jan 17 2022, 5:57 AM.

Details

Summary

The early-claim hack was removed in 96f5cc1ee417f863f85756d1e56b1bed1bd76a7e,
we see a regression about captured var-decl in lambda, see https://github.com/clangd/clangd/issues/990.

Diff Detail

Event Timeline

hokein created this revision.Jan 17 2022, 5:57 AM
hokein requested review of this revision.Jan 17 2022, 5:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 17 2022, 5:57 AM
sammccall accepted this revision.Jan 17 2022, 6:11 AM
sammccall added inline comments.
clang-tools-extra/clangd/Selection.cpp
836

I don't think this is an e.g., but rather is suitable for specifically this case.

If there are other cases, we likely should be testing DeclaratorDecl here, and excluding Constructor/Destructor as before.

852

I wonder whether we want to keep the CXXConstructExpr hack now, up to you.

This revision is now accepted and ready to land.Jan 17 2022, 6:11 AM
hokein updated this revision to Diff 400763.Jan 18 2022, 1:11 AM

refine the comment.

hokein marked an inline comment as done.Jan 18 2022, 1:13 AM
hokein added inline comments.
clang-tools-extra/clangd/Selection.cpp
836

refined the comment.

Testing DeclaratorDecl is safer, but we miss opportunities to know other cases (hopefully, there should be no other cases). For now, I'm leaning towards merely handling this specific case. If it turns out more cases, we can definitely use the DeclaratorDecl.

852

if we remove it, than we need to add the CXXCtorInitializer case back to the earlySourceRange :(

This revision was landed with ongoing or failed builds.Jan 18 2022, 1:22 AM
This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.