[clang] b8ceb9f - [C++20] Diagnose invalid and reserved module names
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 3 05:30:07 PDT 2022
Author: Aaron Ballman
Date: 2022-11-03T08:29:59-04:00
New Revision: b8ceb9f4e4bdb69b5c3ea1ccf8505fa244ca0a1e
URL: https://github.com/llvm/llvm-project/commit/b8ceb9f4e4bdb69b5c3ea1ccf8505fa244ca0a1e
DIFF: https://github.com/llvm/llvm-project/commit/b8ceb9f4e4bdb69b5c3ea1ccf8505fa244ca0a1e.diff
LOG: [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
Added:
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
clang/test/Modules/reserved-names-system-header-1.cpp
clang/test/Modules/reserved-names-system-header-2.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaModule.cpp
clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm
clang/test/Modules/pair-unambiguous-ctor.cppm
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ebf280f4da4a8..73d7aff9b8910 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
-------------------------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a3da8abde58a6..1b1db765fa7a9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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">,
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 4b01f109fc881..19e2c206375bd 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/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.
diff --git a/clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm b/clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm
index 7d4992a2adce8..99fb2327b2d56 100644
--- a/clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm
+++ b/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 {
diff --git a/clang/test/Modules/pair-unambiguous-ctor.cppm b/clang/test/Modules/pair-unambiguous-ctor.cppm
index 8022f34f3aafa..eb242244260cb 100644
--- a/clang/test/Modules/pair-unambiguous-ctor.cppm
+++ b/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")))
diff --git a/clang/test/Modules/reserved-names-1.cpp b/clang/test/Modules/reserved-names-1.cpp
new file mode 100644
index 0000000000000..fd636ab0630a2
--- /dev/null
+++ b/clang/test/Modules/reserved-names-1.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+// expected-note at 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}}
diff --git a/clang/test/Modules/reserved-names-2.cpp b/clang/test/Modules/reserved-names-2.cpp
new file mode 100644
index 0000000000000..6979e92f37765
--- /dev/null
+++ b/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;
diff --git a/clang/test/Modules/reserved-names-3.cpp b/clang/test/Modules/reserved-names-3.cpp
new file mode 100644
index 0000000000000..b2e155e8d3610
--- /dev/null
+++ b/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;
diff --git a/clang/test/Modules/reserved-names-4.cpp b/clang/test/Modules/reserved-names-4.cpp
new file mode 100644
index 0000000000000..73df48b76de8d
--- /dev/null
+++ b/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;
+
diff --git a/clang/test/Modules/reserved-names-system-header-1.cpp b/clang/test/Modules/reserved-names-system-header-1.cpp
new file mode 100644
index 0000000000000..2db4c08add1d9
--- /dev/null
+++ b/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
diff --git a/clang/test/Modules/reserved-names-system-header-2.cpp b/clang/test/Modules/reserved-names-system-header-2.cpp
new file mode 100644
index 0000000000000..2087f487721cb
--- /dev/null
+++ b/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
More information about the cfe-commits
mailing list