[PATCH] D95536: [clang][sema] Note decl location on missing member

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D95536#2553325 <https://reviews.llvm.org/D95536#2553325>, @tbaeder wrote:

> In D95536#2552258 <https://reviews.llvm.org/D95536#2552258>, @aaron.ballman wrote:
>
>> In D95536#2551332 <https://reviews.llvm.org/D95536#2551332>, @tbaeder wrote:
>>
>>> Any update on this?
>>
>> Thank you for the patch! Do you have some motivating examples of when this would really add clarity to the diagnostic? I'm not opposed to the patch per se, but I'm a bit wary about adding an additional note because that makes the diagnostic output seem that much longer, which makes the salient diagnostics a bit harder to find (colorization in the terminal helps with this, though).
>
> I run into this a lot when looking into a larger, new code base. When adding new code, I often look at surrounding code and infer how e.g. an enum member is called. Take https://godbolt.org/z/Tcoxf5 for example, I could've seen code using `OsType::Unix` and inferred that the OsType for Windows will be called `OsType::Windows`, but it's `Win32`. The next step for me as a human is of course to grep the source code for the declaration of `OsType` and check the members. (On the other hand, a "Foo is not a member of enum FooEnum, existing members are: Bar1, Bar2, Bar3, ..." diagnostic would probably be more useful. But that has its own drawbacks).

Hmm... I feel like the diagnostic should already be sufficient to locate the originating location of the class or namespace and the note is adding a bit more (almost, but not quite) noise, but at the same time, I don't feel so strongly that I'd block the patch. I'd like to hear from other reviewers though.

As for some interesting test cases -- I don't think we should issue the note when the diagnostic already appears within the class declaration itself. e.g.,

  struct S {
    void func();
  
    void other(S s) {
      s.foo(); // error, but no need to point to where S is declared
    }
  };

Another similar case but involving an anonymous class:

  struct S {
    struct {
      void func();
    } t;
  
    void foo() {
      t.blech(); // Should we do anything about this?
    }
  };

A pathological case for C code, which shows another case where I think the note is more noise than anything (not that I expect anyone to ever write this code, so not worth special handling for unless it's fallout from other special handling code that's more useful):

  int func(struct S { int a; } s) {
    return s.b;
  }

Anonymous namespaces:

  namespace foo {
  namespace {
    void func();
  }
  }
  
  void bar() {
    foo::blarg(); // Should point to 'foo'?
  }


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

https://reviews.llvm.org/D95536