[clang] e574833 - Downgrade reserved module identifier error into a warning
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 28 05:53:59 PDT 2023
Author: Aaron Ballman
Date: 2023-03-28T08:53:44-04:00
New Revision: e574833c2bc46a39150156eb973d0efb142bc618
URL: https://github.com/llvm/llvm-project/commit/e574833c2bc46a39150156eb973d0efb142bc618
DIFF: https://github.com/llvm/llvm-project/commit/e574833c2bc46a39150156eb973d0efb142bc618.diff
LOG: Downgrade reserved module identifier error into a warning
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
Differential Revision: https://reviews.llvm.org/D146986
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 882e5069e4539..34cfb8e672ef5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -85,6 +85,13 @@ C++20 Feature Support
(`#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
^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 274f58b1a5ff0..2b6525ca5f866 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -855,8 +855,9 @@ def LargeByValueCopy : DiagGroup<"large-by-value-copy">;
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.
//
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 613e4a5006561..79225036f7f2b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -401,6 +401,8 @@ def warn_reserved_extern_symbol: Warning<
"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 @@ 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 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">;
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 8c120d278d634..40ebc9ccdcf96 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -156,11 +156,15 @@ static bool DiagReservedModuleName(Sema &S, const IdentifierInfo *II,
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 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
(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;
}
diff --git a/clang/test/Modules/reserved-names-1.cpp b/clang/test/Modules/reserved-names-1.cpp
index fd636ab0630a2..a92c2244f1cb6 100644
--- a/clang/test/Modules/reserved-names-1.cpp
+++ b/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 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}} \
+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
More information about the cfe-commits
mailing list