Page MenuHomePhabricator

[clangd] Don't navigate to forward class declaration when go to definition.

Authored by hokein on Jan 2 2018, 2:01 AM.

Diff Detail


Event Timeline

hokein created this revision.Jan 2 2018, 2:01 AM
sammccall accepted this revision.Jan 2 2018, 4:54 AM
sammccall added inline comments.
54 ↗(On Diff #128399)

Can you add some motivation here? e.g. the forward decl example

This revision is now accepted and ready to land.Jan 2 2018, 4:54 AM
hokein updated this revision to Diff 128501.Jan 3 2018, 2:12 AM
hokein marked an inline comment as done.
  • Revise the way of checking definition.
  • Add a test for function declaration.
hokein added a comment.Jan 3 2018, 2:14 AM

With more test, it turns out that ASTNode.OrigD is not always pointed to the definition, so we can't rely on D or ASTNode.OrigD :(.

I revised the way of checking definition, it should works for major cases. Please review it again.

ilya-biryukov added inline comments.Jan 3 2018, 2:33 AM
68 ↗(On Diff #128501)

That seems like a useful helper on its own, maybe create a helper called Decl* getDefinition(Decl* D) and use it instead?
It's implementation can be as short as the one we currently have for IsDefinition, since all interesting Decl types have getDefinition method.

hokein updated this revision to Diff 128516.Jan 3 2018, 4:25 AM

Use getDefinition to simply the code.

hokein added inline comments.Jan 3 2018, 4:26 AM
68 ↗(On Diff #128501)

Good point!

This revision was automatically updated to reflect the committed changes.