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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers planned changes to this revision.
nickdesaulniers added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:3685
+            << NRA;
+        notePreviousDefinition(Old, New->getLocation());
+      }
----------------
aaron.ballman wrote:
> 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?
Looks like we do that for `WeakImportAttr`, too. Will fix all cases and amend description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107613