Page MenuHomePhabricator

[clangd] Go-to-definition on 'override' jumps to overridden method(s)
ClosedPublic

Authored by sammccall on Jan 24 2020, 9:21 AM.

Diff Detail

Event Timeline

sammccall created this revision.Jan 24 2020, 9:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2020, 9:21 AM

Unit tests: fail. 62170 tests passed, 5 failed and 813 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 1 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

please fix clang-format and tidy warnings

clang-tools-extra/clangd/XRefs.cpp
287

when do we have OverrideAttribute but no overriden_methods ? It looks like

struct Foo { virtual void foo(); };
struct Bar {
  void foo() override;
  void bar() override;
};

Bar::bar doesn't seem to have OverrideAttr set for example:

|-CXXMethodDecl 0x8e57e88 <line:6:3, col:14> col:8 bar 'void ()'
|-CXXMethodDecl 0x8e57fa0 <line:7:3, col:14> col:8 foo 'void ()'
| |-Overrides: [ 0x8e57668 Foo::foo 'void ()' ]
  | `-OverrideAttr 0x8e58040 <col:14>
sammccall updated this revision to Diff 240819.Tue, Jan 28, 3:04 AM
sammccall marked an inline comment as done.

address comment and clang-format

(the clang-tidy check doesn't reflect a style guideline and I'm not sure it's a good idea, raised on D72217 which added it)

clang-tools-extra/clangd/XRefs.cpp
287

templated code: if foo<t>::bar() overrides base<t>::bar() then the list is empty.
But I think returning "no definition available" is fine, so removed this.

Unit tests: pass. 62255 tests passed, 0 failed and 827 were skipped.

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 1 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

kadircet accepted this revision.Tue, Jan 28, 4:36 AM

Thanks, LGTM

This revision is now accepted and ready to land.Tue, Jan 28, 4:36 AM
This revision was automatically updated to reflect the committed changes.