Page MenuHomePhabricator

[clangd] Show parameters for construct.
ClosedPublic

Authored by lh123 on Nov 26 2021, 2:49 AM.

Details

Summary

Show parameters for construct.

Diff Detail

Event Timeline

lh123 created this revision.Nov 26 2021, 2:49 AM
lh123 requested review of this revision.Nov 26 2021, 2:49 AM
kadircet added inline comments.Nov 26 2021, 8:21 AM
clang-tools-extra/clangd/Hover.cpp
1074

it's a subtle invariant that we only have parameters for functions (which has a return type) or constructor/destructor/conversion-operators (which doesn't have either a type or a return type).

I think we should assert on Type not being present here, as otherwise we would probably duplicate parameters in both places. can you also add a condition around !Type and have a comment saying // Don't print parameters after Type, as they're likely to be mentioned there.

lh123 updated this revision to Diff 390143.Nov 26 2021, 11:31 PM

address comment.

lh123 marked an inline comment as done.Nov 26 2021, 11:35 PM
lh123 added inline comments.
clang-tools-extra/clangd/Hover.cpp
1074

We cannot assert here because the function type has both ReturnType, Type, and Parameters. I think we only need to not print Type when ReturnType or Parameters are present.

kadircet accepted this revision.Dec 2 2021, 5:32 AM

thanks LGTM!
let me know if I should land this for you.

clang-tools-extra/clangd/Hover.cpp
1054

can you add braces around here?

1074

sorry by asserting i meant having it inside the check. this LG, thanks!

1075

can you move the comment above the if and also include as this will just duplicate the information.

This revision is now accepted and ready to land.Dec 2 2021, 5:32 AM
lh123 updated this revision to Diff 391537.Dec 2 2021, 8:36 PM
lh123 marked 3 inline comments as done.

address comment.

This revision was automatically updated to reflect the committed changes.