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@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
