aaron.ballman updated this revision to Diff 471628.
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.
Updating based on review feedback.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136953/new/
https://reviews.llvm.org/D136953
Files:
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
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-system-header-1.cpp
clang/test/Modules/reserved-names-system-header-2.cpp
Index: clang/test/Modules/reserved-names-system-header-2.cpp
===================================================================
--- /dev/null
+++ 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
Index: clang/test/Modules/reserved-names-system-header-1.cpp
===================================================================
--- /dev/null
+++ 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
Index: clang/test/Modules/reserved-names-3.cpp
===================================================================
--- /dev/null
+++ 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;
Index: clang/test/Modules/reserved-names-2.cpp
===================================================================
--- /dev/null
+++ 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' a reserved identifier if it
+// is not the first part of the path.
+export module should_succeed.std0;
Index: clang/test/Modules/reserved-names-1.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/reserved-names-1.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+// expected-note@1 14{{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 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}}
Index: clang/test/Modules/pair-unambiguous-ctor.cppm
===================================================================
--- clang/test/Modules/pair-unambiguous-ctor.cppm
+++ 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 @@
//--- 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")))
Index: clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm
===================================================================
--- clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm
+++ clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm
@@ -6,7 +6,9 @@
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 {
Index: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -144,6 +144,37 @@
TU->setLocalOwningModule(Mod);
}
+/// Tests whether the given identifier is reserved as a module name and
+/// diagnoses if it is. Returs 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 @@
}
}
+ // 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.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11205,6 +11205,8 @@
"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">,
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -344,6 +344,9 @@
potentially problematic function type casts.
- Clang will now disambiguate NTTP types when printing diagnostic that contain NTTP types.
Fixes `Issue 57562 <https://github.com/llvm/llvm-project/issues/57562>`_.
+- Clang now diagnoses use of invalid or reserved module names. 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
-------------------------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
