[clang] c1f7636 - [C++20] [Modules] Continue parsing after we found reserved module names

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 13 00:14:59 PDT 2023


Author: Chuanqi Xu
Date: 2023-04-13T15:14:34+08:00
New Revision: c1f76363e0db41ab6eb9ebedd687ee098491e9b7

URL: https://github.com/llvm/llvm-project/commit/c1f76363e0db41ab6eb9ebedd687ee098491e9b7
DIFF: https://github.com/llvm/llvm-project/commit/c1f76363e0db41ab6eb9ebedd687ee098491e9b7.diff

LOG: [C++20] [Modules] Continue parsing after we found reserved module names

Close https://github.com/llvm/llvm-project/issues/62112

In the previous change, we'll stop parsing directly after we found
reserved module names. But this may be too aggressive. This patch
changes this. Note that the parsing will still be stopped if the module
name is `module` or `import`.

Added: 
    clang/test/Modules/reserved-names-1.cppm
    clang/test/Modules/reserved-names-2.cppm
    clang/test/Modules/reserved-names-3.cppm
    clang/test/Modules/reserved-names-4.cppm

Modified: 
    clang/lib/Sema/SemaModule.cpp

Removed: 
    clang/test/Modules/reserved-names-1.cpp
    clang/test/Modules/reserved-names-2.cpp
    clang/test/Modules/reserved-names-3.cpp
    clang/test/Modules/reserved-names-4.cpp


################################################################################
diff  --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 6c39cc0b44ca4..84a1fd854d804 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -162,7 +162,8 @@ static bool DiagReservedModuleName(Sema &S, const IdentifierInfo *II,
   case Invalid:
     return S.Diag(Loc, diag::err_invalid_module_name) << II;
   case Reserved:
-    return S.Diag(Loc, diag::warn_reserved_module_name) << II;
+    S.Diag(Loc, diag::warn_reserved_module_name) << II;
+    return false;
   }
   llvm_unreachable("fell off a fully covered switch");
 }
@@ -267,10 +268,8 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
   if (!getSourceManager().isInSystemHeader(Path[0].second) &&
       (FirstComponentName == "std" ||
        (FirstComponentName.startswith("std") &&
-        llvm::all_of(FirstComponentName.drop_front(3), &llvm::isDigit)))) {
+        llvm::all_of(FirstComponentName.drop_front(3), &llvm::isDigit))))
     Diag(Path[0].second, diag::warn_reserved_module_name) << Path[0].first;
-    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.

diff  --git a/clang/test/Modules/reserved-names-1.cpp b/clang/test/Modules/reserved-names-1.cpp
deleted file mode 100644
index a92c2244f1cb6..0000000000000
--- a/clang/test/Modules/reserved-names-1.cpp
+++ /dev/null
@@ -1,46 +0,0 @@
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %s
-
-// expected-note at 1 15{{add 'module;' to the start of the file to introduce a global module fragment}}
-
-module std;    // loud-warning {{'std' is a reserved name for a module}}
-module _Test;  // loud-warning {{'_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;   // loud-warning {{'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;  // loud-warning {{'_Test' is a reserved name for a module}} \
-                         expected-error {{module declaration must occur at the start of the translation unit}}
-export module __test; // loud-warning {{'__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; // loud-warning {{'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;    // loud-warning {{'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;// loud-warning {{'std' is a reserved name for a module}} \
-                         expected-error {{module declaration must occur at the start of the translation unit}}
-export module std0;   // loud-warning {{'std0' is a reserved name for a module}} \
-                         expected-error {{module declaration must occur at the start of the translation unit}}
-export module std1000000; // loud-warning {{'std1000000' is a reserved name for a module}} \
-                         expected-error {{module declaration must occur at the start of the translation unit}}
-export module should_diag._Test; // loud-warning {{'_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}}

diff  --git a/clang/test/Modules/reserved-names-1.cppm b/clang/test/Modules/reserved-names-1.cppm
new file mode 100644
index 0000000000000..e780f1e35b3b7
--- /dev/null
+++ b/clang/test/Modules/reserved-names-1.cppm
@@ -0,0 +1,154 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/module.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/module.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/import.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/import.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/_Test.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/_Test.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/__test.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/__test.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/te__st.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/te__st.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/std.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.foo.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/std.foo.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std0.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/std0.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std1000000.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/std1000000.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/module.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/module.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/import.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/import.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/_Test.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/_Test.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/__test.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/__test.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/te__st.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/te__st.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/std.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.foo.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/std.foo.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std0.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/std0.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std1000000.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/std1000000.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/should_diag._Test.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/should_diag._Test.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/system-module.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/system-module.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/system._Test.import.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/system._Test.import.cppm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/user.cpp
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/user.cpp
+
+//--- module.cpp
+module module;  // expected-error {{'module' is an invalid name for a module}}
+
+//--- import.cpp
+module import;  // expected-error {{'import' is an invalid name for a module}}
+
+//--- _Test.cpp
+module _Test;   // loud-warning {{'_Test' is a reserved name for a module}}  \
+                // expected-error {{module '_Test' not found}}
+
+//--- __test.cpp
+module __test; // loud-warning {{'__test' is a reserved name for a module}} \
+               // expected-error {{module '__test' not found}}
+
+//--- te__st.cpp
+module te__st; // loud-warning {{'te__st' is a reserved name for a module}} \
+               // expected-error {{module 'te__st' not found}}
+
+//--- std.cpp
+module std; // loud-warning {{'std' is a reserved name for a module}} \
+            // expected-error {{module 'std' not found}}
+
+//--- std.foo.cpp
+module std.foo; // loud-warning {{'std' is a reserved name for a module}} \
+                // expected-error {{module 'std.foo' not found}}
+
+//--- std0.cpp
+module std0; // loud-warning {{'std0' is a reserved name for a module}} \
+             // expected-error {{module 'std0' not found}}
+
+//--- std1000000.cpp
+module std1000000; // loud-warning {{'std1000000' is a reserved name for a module}} \
+                   // expected-error {{module 'std1000000' not found}}
+
+//--- module.cppm
+export module module; // expected-error {{'module' is an invalid name for a module}}
+
+//--- import.cppm
+export module import; // expected-error {{'import' is an invalid name for a module}}
+
+//--- _Test.cppm
+#ifdef NODIAGNOSTICS
+// expected-no-diagnostics
+#endif
+export module _Test;  // loud-warning {{'_Test' is a reserved name for a module}}
+
+//--- __test.cppm
+#ifdef NODIAGNOSTICS
+// expected-no-diagnostics
+#endif
+export module __test; // loud-warning {{'__test' is a reserved name for a module}}
+export int a = 43;
+
+//--- te__st.cppm
+#ifdef NODIAGNOSTICS
+// expected-no-diagnostics
+#endif
+export module te__st; // loud-warning {{'te__st' is a reserved name for a module}}
+export int a = 43;
+
+//--- std.cppm
+#ifdef NODIAGNOSTICS
+// expected-no-diagnostics
+#endif
+export module std;    // loud-warning {{'std' is a reserved name for a module}}
+
+export int a = 43;
+
+//--- std.foo.cppm
+#ifdef NODIAGNOSTICS
+// expected-no-diagnostics
+#endif
+export module std.foo;// loud-warning {{'std' is a reserved name for a module}}
+
+//--- std0.cppm
+#ifdef NODIAGNOSTICS
+// expected-no-diagnostics
+#endif
+export module std0;   // loud-warning {{'std0' is a reserved name for a module}}
+
+//--- std1000000.cppm
+#ifdef NODIAGNOSTICS
+// expected-no-diagnostics
+#endif
+export module std1000000; // loud-warning {{'std1000000' is a reserved name for a module}}
+
+//--- should_diag._Test.cppm
+#ifdef NODIAGNOSTICS
+// expected-no-diagnostics
+#endif
+export module should_diag._Test; // loud-warning {{'_Test' is a reserved name for a module}}
+
+//--- system-module.cppm
+// 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}}
+
+//--- system._Test.import.cppm
+# 34 "reserved-names-1.cpp" 1 3
+export module _Test.import; // expected-error {{'import' is an invalid name for a module}}
+
+//--- user.cpp
+// We can still use a reserved name on imoport.
+import std; // expected-error {{module 'std' not found}}

diff  --git a/clang/test/Modules/reserved-names-2.cpp b/clang/test/Modules/reserved-names-2.cppm
similarity index 100%
rename from clang/test/Modules/reserved-names-2.cpp
rename to clang/test/Modules/reserved-names-2.cppm

diff  --git a/clang/test/Modules/reserved-names-3.cpp b/clang/test/Modules/reserved-names-3.cppm
similarity index 100%
rename from clang/test/Modules/reserved-names-3.cpp
rename to clang/test/Modules/reserved-names-3.cppm

diff  --git a/clang/test/Modules/reserved-names-4.cpp b/clang/test/Modules/reserved-names-4.cppm
similarity index 100%
rename from clang/test/Modules/reserved-names-4.cpp
rename to clang/test/Modules/reserved-names-4.cppm


        


More information about the cfe-commits mailing list