[C++20] Diagnose invalid and reserved module names

[module.unit]p1 specifies that module and import are invalid components
of a module name, that module names cannot contain reserved
identifiers, and that std followed by zero or more digits is reserved.

The first issue (module and import pseudo-keywords) requires a
diagnostic, the second issue (use of reserved identifiers) does not
require a diagnostic. We diagnose both the same -- the code is ill-
formed unless the module declaration is in a system "header". This
allows STL implementations to use the reserved module names while
preventing users from stealing them out from under us.

Differential Revision: https://reviews.llvm.org/D136953

clang/docs/ReleaseNotes.rst

@@ -377,6 +377,9 @@ Improvements to Clang's diagnostics
   `Issue 58673 <https://github.com/llvm/llvm-project/issues/58673>`_.
 - Better diagnostics when the user has missed `auto` in a declaration.
   `Issue 49129 <https://github.com/llvm/llvm-project/issues/49129>`_.
+- Clang now diagnoses use of invalid or reserved module names in a module
+  export declaration. Both are diagnosed as an error, but the diagnostic is
+  suppressed for use of reserved names in a system header.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -11207,6 +11207,8 @@ def err_private_module_fragment_not_module_interface : Error<
   "private module fragment in module implementation unit">;
 def note_not_module_interface_add_export : Note<
   "add 'export' here if this is intended to be a module interface unit">;
+def err_invalid_module_name : Error<
+  "%0 is %select{an invalid|a reserved}1 name for a module">;
 
 def ext_equivalent_internal_linkage_decl_in_modules : ExtWarn<
   "ambiguous use of internal linkage declaration %0 defined in multiple modules">,

clang/lib/Sema/SemaModule.cpp

@@ -144,6 +144,37 @@ void Sema::HandleStartOfHeaderUnit() {
   TU->setLocalOwningModule(Mod);
 }
 
+/// Tests whether the given identifier is reserved as a module name and
+/// diagnoses if it is. Returns true if a diagnostic is emitted and false
+/// otherwise.
+static bool DiagReservedModuleName(Sema &S, const IdentifierInfo *II,
+                                   SourceLocation Loc) {
+  enum {
+    Valid = -1,
+    Invalid = 0,
+    Reserved = 1,
+  } Reason = Valid;
+
+  StringRef PartName = II->getName();
+  if (II->isStr("module") || II->isStr("import"))
+    Reason = Invalid;
+  else if (II->isReserved(S.getLangOpts()) !=
+           ReservedIdentifierStatus::NotReserved)
+    Reason = Reserved;
+
+  // If the identifier is reserved (not invalid) but is in a system header,
+  // we do not diagnose (because we expect system headers to use reserved
+  // identifiers).
+  if (Reason == Reserved && S.getSourceManager().isInSystemHeader(Loc))
+    Reason = Valid;
+
+  if (Reason != Valid) {
+    S.Diag(Loc, diag::err_invalid_module_name) << II << (int)Reason;
+    return true;
+  }
+  return false;
+}
+
 Sema::DeclGroupPtrTy
 Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
                       ModuleDeclKind MDK, ModuleIdPath Path,
@@ -238,6 +269,32 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
     }
   }
 
+  // C++2b [module.unit]p1: ... The identifiers module and import shall not
+  // appear as identifiers in a module-name or module-partition. All
+  // module-names either beginning with an identifier consisting of std
+  // followed by zero or more digits or containing a reserved identifier
+  // ([lex.name]) are reserved and shall not be specified in a
+  // module-declaration; no diagnostic is required.
+
+  // Test the first part of the path to see if it's std[0-9]+ but allow the
+  // name in a system header.
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+      (FirstComponentName == "std" ||
+       (FirstComponentName.startswith("std") &&
+        llvm::all_of(FirstComponentName.drop_front(3), &llvm::isDigit)))) {
+    Diag(Path[0].second, diag::err_invalid_module_name)
+        << Path[0].first << /*reserved*/ 1;
+    return nullptr;
+  }
+
+  // Then test all of the components in the path to see if any of them are
+  // using another kind of reserved or invalid identifier.
+  for (auto Part : Path) {
+    if (DiagReservedModuleName(*this, Part.first, Part.second))
+      return nullptr;
+  }
+
   // Flatten the dots in a module name. Unlike Clang's hierarchical module map
   // modules, the dots here are just another character that can appear in a
   // module name.

clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm

@@ -6,7 +6,9 @@ class Pooh;
 class Piglet;
 # 8 "" 2
 
+# 8 "" 1 3
 export module std; // might happen, you can't say it won't!
+# 9 "" 2 3
 
 namespace std {
 export template<typename T> class allocator {

clang/test/Modules/pair-unambiguous-ctor.cppm

@@ -14,7 +14,9 @@
 // expected-no-diagnostics
 module;
 #include "config.h"
+# 3 "pair-unambiguous-ctor.cppm" 1 3
 export module std:M;
+# 3 "pair-unambiguous-ctor.cppm" 2 3
 import :string;
 import :algorithm;
 
@@ -25,15 +27,19 @@ auto check() {
 //--- string.cppm
 module;
 #include "string.h"
+# 28 "pair-unambiguous-ctor.cppm" 1 3
 export module std:string;
 export namespace std {
     using std::string;
 }
+# 28 "pair-unambiguous-ctor.cppm" 2 3
 
 //--- algorithm.cppm
 module;
 #include "algorithm.h"
+# 38 "pair-unambiguous-ctor.cppm" 1 3
 export module std:algorithm;
+# 38 "pair-unambiguous-ctor.cppm" 2 3
 
 //--- pair.h
 namespace std __attribute__ ((__visibility__ ("default")))

clang/test/Modules/reserved-names-1.cpp

@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+// expected-note@1 15{{add 'module;' to the start of the file to introduce a global module fragment}}
+
+module std;    // expected-error {{'std' is a reserved name for a module}}
+module _Test;  // expected-error {{'_Test' is a reserved name for a module}} \
+                  expected-error {{module declaration must occur at the start of the translation unit}}
+module module; // expected-error {{'module' is an invalid name for a module}} \
+                  expected-error {{module declaration must occur at the start of the translation unit}}
+module std0;   // expected-error {{'std0' is a reserved name for a module}} \
+                  expected-error {{module declaration must occur at the start of the translation unit}}
+
+export module module; // expected-error {{'module' is an invalid name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module import; // expected-error {{'import' is an invalid name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module _Test;  // expected-error {{'_Test' is a reserved name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module __test; // expected-error {{'__test' is a reserved name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module te__st; // expected-error {{'te__st' is a reserved name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module std;    // expected-error {{'std' is a reserved name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module std.foo;// expected-error {{'std' is a reserved name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module std0;   // expected-error {{'std0' is a reserved name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module std1000000; // expected-error {{'std1000000' is a reserved name for a module}} \
+                         expected-error {{module declaration must occur at the start of the translation unit}}
+export module should_fail._Test; // expected-error {{'_Test' is a reserved name for a module}} \
+                                    expected-error {{module declaration must occur at the start of the translation unit}}
+
+// Show that being in a system header doesn't save you from diagnostics about
+// use of an invalid module-name identifier.
+# 34 "reserved-names-1.cpp" 1 3
+export module module;       // expected-error {{'module' is an invalid name for a module}} \
+                               expected-error {{module declaration must occur at the start of the translation unit}}
+
+export module _Test.import; // expected-error {{'import' is an invalid name for a module}} \
+                               expected-error {{module declaration must occur at the start of the translation unit}}
+# 39 "reserved-names-1.cpp" 2 3
+
+// We can still use a reserved name on imoport.
+import std; // expected-error {{module 'std' not found}}

clang/test/Modules/reserved-names-2.cpp

@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// Demonstrate that we don't consider use of 'std' followed by digits to be a
+// reserved identifier if it is not the first part of the path.
+export module should_succeed.std0;

clang/test/Modules/reserved-names-3.cpp

@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// Demonstrate that we don't consider use of 'std' (potentially followed by
+// zero or more digits) to be a reserved identifier if it is not the only part
+// of the path.
+export module std12Three;

clang/test/Modules/reserved-names-4.cpp

@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// Demonstrate that we don't consider use of 'std' a reserved identifier if it
+// is not the first part of the path.
+export module should_succeed.std;
+

clang/test/Modules/reserved-names-system-header-1.cpp

@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// Show that we suppress the reserved identifier diagnostic in a system header.
+# 100 "file.cpp" 1 3  // Enter a system header
+export module std;
+# 100 "file.cpp" 2 3  // Leave the system header

clang/test/Modules/reserved-names-system-header-2.cpp

@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// Show that we suppress the reserved identifier diagnostic in a system header.
+# 100 "file.cpp" 1 3  // Enter a system header
+export module __test;
+# 100 "file.cpp" 2 3  // Leave the system header