[Clang] Reject programs declaring namespace std to be inline

[rZhBoYao]

Summary

Fixes #64041

[rZhBoYao]

https://eel.is/c++draft/namespace.std#7 is in the library clause.
Couldn't find a better place to put the test other than clang/test/CXX/dcl.dcl/basic.namespace/namespace.def/p7.cpp which is for https://eel.is/c++draft/namespace.def.general#4.sentence-2.

[cor3ntin]

Can you add a test for foo::std ? I suspects it warns, which is incorrect ? Can you add additional tests for inline std::foo ? (which should warn)
Thanks!

[MitalAshok]

This does currently break namespace foo { inline namespace std {} }, namespace foo::inline std {}, etc.

clang/lib/Sema/SemaDeclCXX.cpp

11396

You need to check if this is a std namespace declaration at file scope, namespace foo::inline std {} in a namespace scope should be fine.

The check for this is a few lines below: CurContext->getRedeclContext()->isTranslationUnit().

11429–11430

This ends up giving two errors on the same line if this wasn't the first declaration (error: namespace 'std' cannot be declared inline followed by error: non-inline namespace cannot be reopened as inline; note: previous definition is here). The wording for the second error is also a little confusing (it cannot be opened at all as inline, let alone reopened), so consider refactoring so that both diagnostics can't be issued at once

[rZhBoYao]

Addressed comments

[MitalAshok]

Looks good!

[rZhBoYao]

Thanks for all the suggestions and review comments!

clang/docs/ReleaseNotes.rst

 
 C++ Language Changes
 --------------------
 - Improved ``-O0`` code generation for calls to ``std::forward_like``. Similarly to
   ``std::move, std::forward`` et al. it is now treated as a compiler builtin and implemented
   directly rather than instantiating the definition from the standard library.
 - Implemented `CWG2518 <https://wg21.link/CWG2518>`_ which allows ``static_assert(false)``
   to not be ill-formed when its condition is evaluated in the context of a template definition.
+- Declaring namespace std to be an inline namespace is now prohibited, `[namespace.std]p7`.
 
 C++20 Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 - Implemented the rule introduced by `CA104 <https://wg21.link/P2103R0>`_  for comparison of
   constraint-expressions. Improved support for out-of-line definitions of constrained templates.
   This fixes:
   `#49620 <https://github.com/llvm/llvm-project/issues/49620>`_,
   `#60231 <https://github.com/llvm/llvm-project/issues/60231>`_,

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_cxx14_compat_inline_variable : Warning<
   "inline variables are incompatible with C++ standards before C++17">,
   DefaultIgnore, InGroup<CXXPre17Compat>;
 
 def warn_inline_namespace_reopened_noninline : Warning<
   "inline namespace reopened as a non-inline namespace">,
   InGroup<InlineNamespaceReopenedNoninline>;
 def err_inline_namespace_mismatch : Error<
   "non-inline namespace cannot be reopened as inline">;
+def err_inline_namespace_std : Error<
+  "cannot declare the namespace 'std' to be inline">;
 
 def err_unexpected_friend : Error<
   "friends can only be classes or functions">;
 def ext_enum_friend : ExtWarn<
   "befriending enumeration type %0 is a C++11 extension">, InGroup<CXX11>;
 def warn_cxx98_compat_enum_friend : Warning<
   "befriending enumeration type %0 is incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;

clang/lib/Sema/SemaDeclCXX.cpp

Decl *Sema::ActOnStartNamespaceDef(Scope *NamespcScope,
   bool IsInline = InlineLoc.isValid();
   bool IsInvalid = false;
   bool IsStd = false;
   bool AddToKnown = false;
   Scope *DeclRegionScope = NamespcScope->getParent();
 
   NamespaceDecl *PrevNS = nullptr;
   if (II) {
+    // C++ [namespace.std]p7:
+    //   A translation unit shall not declare namespace std to be an inline
+    //   namespace (9.8.2).
+    //
+    // Precondition: the std namespace is in the file scope and is declared to
+    // be inline
+    auto DiagnoseInlineStdNS = [&]() {
+      assert(IsInline && II->isStr("std") &&
+             CurContext->getRedeclContext()->isTranslationUnit() &&
+             "Precondition of DiagnoseInlineStdNS not met");
+      Diag(InlineLoc, diag::err_inline_namespace_std)
+          << SourceRange(InlineLoc, InlineLoc.getLocWithOffset(6));
+      IsInline = false;
+    };
     // C++ [namespace.def]p2:
     //   The identifier in an original-namespace-definition shall not
     //   have been previously defined in the declarative region in
     //   which the original-namespace-definition appears. The
     //   identifier in an original-namespace-definition is the name of
     //   the namespace. Subsequently in that declarative region, it is
     //   treated as an original-namespace-name.
     //
     // Since namespace names are unique in their scope, and we don't
     // look through using directives, just look for any ordinary names
     // as if by qualified name lookup.
     LookupResult R(*this, II, IdentLoc, LookupOrdinaryName,
                    ForExternalRedeclaration);
     LookupQualifiedName(R, CurContext->getRedeclContext());
     NamedDecl *PrevDecl =
         R.isSingleResult() ? R.getRepresentativeDecl() : nullptr;
     PrevNS = dyn_cast_or_null<NamespaceDecl>(PrevDecl);
 
     if (PrevNS) {
       // This is an extended namespace definition.
-      if (IsInline != PrevNS->isInline())
+      if (IsInline && II->isStr("std") &&
+          CurContext->getRedeclContext()->isTranslationUnit())
+        DiagnoseInlineStdNS();
+      else if (IsInline != PrevNS->isInline())
         DiagnoseNamespaceInlineMismatch(*this, NamespaceLoc, Loc, II,
                                         &IsInline, PrevNS);
     } else if (PrevDecl) {
       // This is an invalid name redefinition.
       Diag(Loc, diag::err_redefinition_different_kind)
         << II;
       Diag(PrevDecl->getLocation(), diag::note_previous_definition);
       IsInvalid = true;
       // Continue on to push Namespc as current DeclContext and return it.
     } else if (II->isStr("std") &&
                CurContext->getRedeclContext()->isTranslationUnit()) {
+      if (IsInline)
+        DiagnoseInlineStdNS();
       // This is the first "real" definition of the namespace "std", so update
       // our cache of the "std" namespace to point at this definition.
       PrevNS = getStdNamespace();
       IsStd = true;
       AddToKnown = !IsInline;
     } else {
       // We've seen this namespace for the first time.
       AddToKnown = !IsInline;

clang/test/CXX/dcl.dcl/basic.namespace/namespace.def/p7.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 // FIXME: We should probably suppress the warning on reopening an inline
 // namespace without the inline keyword if it's not the first opening of the
 // namespace in the file, because there's no risk of the inlineness differing
 // across TUs in that case.
 
 namespace NIL {} // expected-note {{previous definition}}
 inline namespace NIL {} // expected-error {{cannot be reopened as inline}}
 inline namespace IL {} // expected-note {{previous definition}}
 namespace IL {} // expected-warning{{inline namespace reopened as a non-inline namespace}}
 
 namespace {} // expected-note {{previous definition}}
 inline namespace {} // expected-error {{cannot be reopened as inline}}
 namespace X {
   inline namespace {} // expected-note {{previous definition}}
   namespace {} // expected-warning {{inline namespace reopened as a non-inline namespace}}
 }
+
+namespace std {}
+inline namespace std {} // expected-error{{cannot declare the namespace 'std' to be inline}}
+inline namespace std::foo {} // expected-error{{cannot declare the namespace 'std' to be inline}}
+                             // expected-error@-1{{nested namespace definition cannot be 'inline'}}
+namespace foo::inline std {} // expected-note {{previous definition}}
+namespace foo { inline namespace std {} } // OK
+namespace foo { namespace std {} } // expected-warning {{inline namespace reopened as a non-inline namespace}}