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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(the clang-tidy check doesn't reflect a style guideline and I'm not sure it's a good idea, raised on D72217 <https://reviews.llvm.org/D72217> which added it)



================
Comment at: clang-tools-extra/clangd/XRefs.cpp:287
+        // We may be overridding multiple methods - offer them all.
+        for (const NamedDecl* ND : CMD->overridden_methods()) {
+          AddResultDecl(ND);
----------------
kadircet wrote:
> 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>
> ```
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73367/new/

https://reviews.llvm.org/D73367