aaron.ballman created this revision.
aaron.ballman added reviewers: ChuanqiXu, dblaikie, Mordante, iains.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.
Any project that wants to `import std;` potentially needs to be able to build a
module that does `export std;`. We silenced the error diagnostic if the module
identified itself as a system header, but this isn't quite good enough, what we
really need is a way to identify a system module. It would be nice for that
feature to be shared among the major implementations, so this downgrades the
diagnostic from an error to a warning temporarily to give implementers time to
determine what that mechanism will look like. We may convert this warning back
into an error in a future release of Clang, but it's not guaranteed we will do
so.
Fixes https://github.com/llvm/llvm-project/issues/61446
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D146986
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaModule.cpp
clang/test/Modules/reserved-names-1.cpp
Index: clang/test/Modules/reserved-names-1.cpp
===================================================================
--- clang/test/Modules/reserved-names-1.cpp
+++ clang/test/Modules/reserved-names-1.cpp
@@ -1,34 +1,35 @@
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// 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@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}} \
+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; // expected-error {{'std0' is a reserved name for a module}} \
+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; // expected-error {{'_Test' is a reserved name for a module}} \
+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; // expected-error {{'__test' is a reserved name for a module}} \
+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; // expected-error {{'te__st' is a reserved name for a module}} \
+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; // expected-error {{'std' is a reserved name for a module}} \
+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;// expected-error {{'std' is a reserved name for a module}} \
+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; // expected-error {{'std0' is a reserved name for a module}} \
+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; // expected-error {{'std1000000' is a reserved name for a module}} \
+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_fail._Test; // expected-error {{'_Test' is a reserved name for a module}} \
+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
Index: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -156,11 +156,15 @@
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;
+ switch (Reason) {
+ case Valid:
+ return false;
+ case Invalid:
+ return S.Diag(Loc, diag::err_invalid_module_name) << II;
+ case Reserved:
+ return S.Diag(Loc, diag::warn_reserved_module_name) << II;
}
- return false;
+ llvm_unreachable("fell off a fully covered switch");
}
Sema::DeclGroupPtrTy
@@ -264,8 +268,7 @@
(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;
+ Diag(Path[0].second, diag::warn_reserved_module_name) << Path[0].first;
return nullptr;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -401,6 +401,8 @@
"it starts with '_' followed by a capital letter|"
"it contains '__'}1">,
InGroup<ReservedIdentifier>, DefaultIgnore;
+def warn_reserved_module_name : Warning<
+ "%0 is a reserved name for a module">, InGroup<ReservedModuleIdentifier>;
def warn_parameter_size: Warning<
"%0 is a large (%1 bytes) pass-by-value argument; "
@@ -11249,8 +11251,7 @@
"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 err_invalid_module_name : Error<"%0 is an invalid name for a module">;
def err_extern_def_in_header_unit : Error<
"non-inline external definitions are not permitted in C++ header units">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -855,8 +855,9 @@
def DuplicateArgDecl : DiagGroup<"duplicate-method-arg">;
def SignedEnumBitfield : DiagGroup<"signed-enum-bitfield">;
+def ReservedModuleIdentifier : DiagGroup<"reserved-module-identifier">;
def ReservedIdentifier : DiagGroup<"reserved-identifier",
- [ReservedIdAsMacro]>;
+ [ReservedIdAsMacro, ReservedModuleIdentifier]>;
// Unreachable code warning groups.
//
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -85,6 +85,13 @@
(`#61278 <https://github.com/llvm/llvm-project/issues/61278>`_)
- Announced C++20 Coroutines is fully supported on all targets except Windows, which
still has some stability and ABI issues.
+- Downgraded use of a reserved identifier in a module export declaration from
+ an error to a warning under the ``-Wreserved-module-identifier`` warning
+ group. This warning is enabled by default. This addresses `#61446
+ <https://github.com/llvm/llvm-project/issues/61446>`_ and allows easier
+ building of precompiled modules. This diagnostic may be strengthened into an
+ error again in the future once there is a less fragile way to mark a module
+ as being part of the implementation rather than a user module.
C++2b Feature Support
^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
