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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
