[clang] Improve diagnostic for reopened inline namespace

[fwolff]

Summary

Fixes #50794. Reopening an inline namespace without the inline keyword yields a warning:

inline namespace abc {}
namespace abc {}
namespace abc {}

<source>:2:11: warning: inline namespace reopened as a non-inline namespace [-Winline-namespace-reopened-noninline]
namespace abc {}
          ^
inline 
<source>:1:18: note: previous definition is here
inline namespace abc {}
                 ^
<source>:3:11: warning: inline namespace reopened as a non-inline namespace [-Winline-namespace-reopened-noninline]
namespace abc {}
          ^
inline 
<source>:2:11: note: previous definition is here
namespace abc {}
          ^

But you'll notice that the second "previous definition is here" is actually not helpful, because this previous definition isn't explicitly marked inline. I think that the note should point to the first definition instead, which must always have the inline if the namespace is supposed to be an inline namespace.

[cor3ntin]

This looks good to me, thanks!

[aaron.ballman]

Thank you for the fix! This LGTM with a minor nit, but please also add a release note for the fix.

clang/lib/Sema/SemaDeclCXX.cpp

11062–11066

There are no circumstances under which getFirstDecl() should return null, so I think this can be simplified.

clang/lib/Sema/SemaDeclCXX.cpp

 /// Diagnose a mismatch in 'inline' qualifiers when a namespace is
 /// reopened.
 static void DiagnoseNamespaceInlineMismatch(Sema &S, SourceLocation KeywordLoc,
                                             SourceLocation Loc,
                                             IdentifierInfo *II, bool *IsInline,
                                             NamespaceDecl *PrevNS) {
   assert(*IsInline != PrevNS->isInline());
 
+  if (auto *FirstNS = PrevNS->getFirstDecl())
+    // 'inline' must appear on the original definition, but not necessarily
+    // on all extension definitions, so the note should point to the first
+    // definition to avoid confusion.
+    PrevNS = FirstNS;
+
   if (PrevNS->isInline())
     // The user probably just forgot the 'inline', so suggest that it
     // be added back.
     S.Diag(Loc, diag::warn_inline_namespace_reopened_noninline)
       << FixItHint::CreateInsertion(KeywordLoc, "inline ");
   else
     S.Diag(Loc, diag::err_inline_namespace_mismatch);
 

clang/test/Parser/cxx2a-inline-nested-namespace-definition.cpp

 // expected-warning@+7 {{nested namespace definition is a C++17 extension; define each namespace separately}}
 // expected-warning@+6 {{inline nested namespace definition is a C++20 extension}}
 #elif __cplusplus <= 201703L
 // expected-warning@+4 {{inline nested namespace definition is a C++20 extension}}
 #else
 // expected-warning@+2 {{inline nested namespace definition is incompatible with C++ standards before C++20}}
 #endif
 namespace valid1::valid2::inline valid3::inline valid4::valid5 {}
-// expected-note@-1 2 {{previous definition is here}}
+// expected-note@-1 4 {{previous definition is here}}
 
 #if __cplusplus <= 201402L
 // expected-warning@+3 {{nested namespace definition is a C++17 extension; define each namespace separately}}
 #endif
 //expected-warning@+1 2 {{inline namespace reopened as a non-inline namespace}}
 namespace valid1::valid2::valid3::valid4::valid5 {}
 
 #if __cplusplus <= 201402L
 // expected-warning@+7 {{nested namespace definition is a C++17 extension; define each namespace separately}}
 // expected-warning@+6 {{inline nested namespace definition is a C++20 extension}}
 #elif __cplusplus <= 201703L
 // expected-warning@+4 {{inline nested namespace definition is a C++20 extension}}
 #else
 // expected-warning@+2 {{inline nested namespace definition is incompatible with C++ standards before C++20}}
 #endif
 namespace valid1::valid2::inline valid3::inline valid4::valid5 {}
-// expected-note@-1 2 {{previous definition is here}}
 
 namespace valid1 {
 namespace valid2 {
 //expected-warning@+1 {{inline namespace reopened as a non-inline namespace}}
 namespace valid3 {
 //expected-warning@+1 {{inline namespace reopened as a non-inline namespace}}
 namespace valid4 {
 namespace valid5 {
 }
 } // namespace valid4
 } // namespace valid3
 } // namespace valid2
 } // namespace valid1
 

clang/test/SemaCXX/warn-inline-namespace-reopened-twice.cpp

+// RUN: %clang_cc1 -fsyntax-only -Wall -verify -std=c++11 %s
+
+// Regression test for #50794.
+
+// expected-note@+1 2 {{previous definition is here}}
+inline namespace X {}
+
+namespace X {} // expected-warning {{inline namespace reopened as a non-inline namespace}}
+namespace X {} // expected-warning {{inline namespace reopened as a non-inline namespace}}