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