[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:3685
+            << NRA;
+        notePreviousDefinition(Old, New->getLocation());
+      }
----------------
nickdesaulniers wrote:
> ah, this isn't quite right.  It replaces warning of the first declaration with warning of the first definition.
> 
> This should instead use
> 
> `Diag(Old->getLocation(), diag::note_previous_declaration);`
Good catch! I think we want to use `note_previous_declaration` in all three cases though, don't we? It's a bit weird that we diagnose "attribute not present on first declaration" but then try to note a definition. e.g., https://godbolt.org/z/jd9oG96sT

I know it's a bit of a drive-by, but I think it's probably worth correcting that as well. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107613