On Thu, Apr 13, 2023 at 3:14 AM Chuanqi Xu via cfe-commits
<cfe-commits@lists.llvm.org> wrote:
>
>
> Author: Chuanqi Xu
> Date: 2023-04-13T15:14:34+08:00
> New Revision: c1f76363e0db41ab6eb9ebedd687ee098491e9b7
>
> URL:
> https://github.com/llvm/llvm-project/commit/c1f76363e0db41ab6eb9ebedd687ee098491e9b7
> DIFF:
> https://github.com/llvm/llvm-project/commit/c1f76363e0db41ab6eb9ebedd687ee098491e9b7.diff
>
> LOG: [C++20] [Modules] Continue parsing after we found reserved module names
>
> Close https://github.com/llvm/llvm-project/issues/62112
>
> In the previous change, we'll stop parsing directly after we found
> reserved module names. But this may be too aggressive. This patch
> changes this. Note that the parsing will still be stopped if the module
> name is `module` or `import`.
Thank you for fixing this up! I think this should be backported to 16.0.2, WDYT?
~Aaron
>
> Added:
> clang/test/Modules/reserved-names-1.cppm
> clang/test/Modules/reserved-names-2.cppm
> clang/test/Modules/reserved-names-3.cppm
> clang/test/Modules/reserved-names-4.cppm
>
> Modified:
> clang/lib/Sema/SemaModule.cpp
>
> Removed:
> 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
>
>
> ################################################################################
> diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
> index 6c39cc0b44ca4..84a1fd854d804 100644
> --- a/clang/lib/Sema/SemaModule.cpp
> +++ b/clang/lib/Sema/SemaModule.cpp
> @@ -162,7 +162,8 @@ static bool DiagReservedModuleName(Sema &S, const
> IdentifierInfo *II,
> case Invalid:
> return S.Diag(Loc, diag::err_invalid_module_name) << II;
> case Reserved:
> - return S.Diag(Loc, diag::warn_reserved_module_name) << II;
> + S.Diag(Loc, diag::warn_reserved_module_name) << II;
> + return false;
> }
> llvm_unreachable("fell off a fully covered switch");
> }
> @@ -267,10 +268,8 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc,
> SourceLocation ModuleLoc,
> if (!getSourceManager().isInSystemHeader(Path[0].second) &&
> (FirstComponentName == "std" ||
> (FirstComponentName.startswith("std") &&
> - llvm::all_of(FirstComponentName.drop_front(3), &llvm::isDigit)))) {
> + llvm::all_of(FirstComponentName.drop_front(3), &llvm::isDigit))))
> Diag(Path[0].second, diag::warn_reserved_module_name) << Path[0].first;
> - 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.
>
> diff --git a/clang/test/Modules/reserved-names-1.cpp
> b/clang/test/Modules/reserved-names-1.cpp
> deleted file mode 100644
> index a92c2244f1cb6..0000000000000
> --- a/clang/test/Modules/reserved-names-1.cpp
> +++ /dev/null
> @@ -1,46 +0,0 @@
> -// 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; // 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; // 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; // 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; // 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; // 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; // 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;// 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; // 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; // 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_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
> -// 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-1.cppm
> b/clang/test/Modules/reserved-names-1.cppm
> new file mode 100644
> index 0000000000000..e780f1e35b3b7
> --- /dev/null
> +++ b/clang/test/Modules/reserved-names-1.cppm
> @@ -0,0 +1,154 @@
> +// RUN: rm -rf %t
> +// RUN: mkdir -p %t
> +// RUN: split-file %s %t
> +
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/module.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/module.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/import.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/import.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/_Test.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/_Test.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/__test.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/__test.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/te__st.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/te__st.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/std.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/std.foo.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/std.foo.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std0.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/std0.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/std1000000.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/std1000000.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/module.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/module.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/import.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/import.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/_Test.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/_Test.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/__test.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/__test.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/te__st.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/te__st.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/std.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/std.foo.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/std.foo.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/std0.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/std0.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/std1000000.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/std1000000.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/should_diag._Test.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS
> -Wno-reserved-module-identifier %t/should_diag._Test.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/system-module.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/system-module.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud
> %t/system._Test.import.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/system._Test.import.cppm
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/user.cpp
> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected
> -Wno-reserved-module-identifier %t/user.cpp
> +
> +//--- module.cpp
> +module module; // expected-error {{'module' is an invalid name for a
> module}}
> +
> +//--- import.cpp
> +module import; // expected-error {{'import' is an invalid name for a
> module}}
> +
> +//--- _Test.cpp
> +module _Test; // loud-warning {{'_Test' is a reserved name for a module}}
> \
> + // expected-error {{module '_Test' not found}}
> +
> +//--- __test.cpp
> +module __test; // loud-warning {{'__test' is a reserved name for a module}} \
> + // expected-error {{module '__test' not found}}
> +
> +//--- te__st.cpp
> +module te__st; // loud-warning {{'te__st' is a reserved name for a module}} \
> + // expected-error {{module 'te__st' not found}}
> +
> +//--- std.cpp
> +module std; // loud-warning {{'std' is a reserved name for a module}} \
> + // expected-error {{module 'std' not found}}
> +
> +//--- std.foo.cpp
> +module std.foo; // loud-warning {{'std' is a reserved name for a module}} \
> + // expected-error {{module 'std.foo' not found}}
> +
> +//--- std0.cpp
> +module std0; // loud-warning {{'std0' is a reserved name for a module}} \
> + // expected-error {{module 'std0' not found}}
> +
> +//--- std1000000.cpp
> +module std1000000; // loud-warning {{'std1000000' is a reserved name for a
> module}} \
> + // expected-error {{module 'std1000000' not found}}
> +
> +//--- module.cppm
> +export module module; // expected-error {{'module' is an invalid name for a
> module}}
> +
> +//--- import.cppm
> +export module import; // expected-error {{'import' is an invalid name for a
> module}}
> +
> +//--- _Test.cppm
> +#ifdef NODIAGNOSTICS
> +// expected-no-diagnostics
> +#endif
> +export module _Test; // loud-warning {{'_Test' is a reserved name for a
> module}}
> +
> +//--- __test.cppm
> +#ifdef NODIAGNOSTICS
> +// expected-no-diagnostics
> +#endif
> +export module __test; // loud-warning {{'__test' is a reserved name for a
> module}}
> +export int a = 43;
> +
> +//--- te__st.cppm
> +#ifdef NODIAGNOSTICS
> +// expected-no-diagnostics
> +#endif
> +export module te__st; // loud-warning {{'te__st' is a reserved name for a
> module}}
> +export int a = 43;
> +
> +//--- std.cppm
> +#ifdef NODIAGNOSTICS
> +// expected-no-diagnostics
> +#endif
> +export module std; // loud-warning {{'std' is a reserved name for a
> module}}
> +
> +export int a = 43;
> +
> +//--- std.foo.cppm
> +#ifdef NODIAGNOSTICS
> +// expected-no-diagnostics
> +#endif
> +export module std.foo;// loud-warning {{'std' is a reserved name for a
> module}}
> +
> +//--- std0.cppm
> +#ifdef NODIAGNOSTICS
> +// expected-no-diagnostics
> +#endif
> +export module std0; // loud-warning {{'std0' is a reserved name for a
> module}}
> +
> +//--- std1000000.cppm
> +#ifdef NODIAGNOSTICS
> +// expected-no-diagnostics
> +#endif
> +export module std1000000; // loud-warning {{'std1000000' is a reserved name
> for a module}}
> +
> +//--- should_diag._Test.cppm
> +#ifdef NODIAGNOSTICS
> +// expected-no-diagnostics
> +#endif
> +export module should_diag._Test; // loud-warning {{'_Test' is a reserved
> name for a module}}
> +
> +//--- system-module.cppm
> +// 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}}
> +
> +//--- system._Test.import.cppm
> +# 34 "reserved-names-1.cpp" 1 3
> +export module _Test.import; // expected-error {{'import' is an invalid name
> for a module}}
> +
> +//--- user.cpp
> +// 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.cppm
> similarity index 100%
> rename from clang/test/Modules/reserved-names-2.cpp
> rename to clang/test/Modules/reserved-names-2.cppm
>
> diff --git a/clang/test/Modules/reserved-names-3.cpp
> b/clang/test/Modules/reserved-names-3.cppm
> similarity index 100%
> rename from clang/test/Modules/reserved-names-3.cpp
> rename to clang/test/Modules/reserved-names-3.cppm
>
> diff --git a/clang/test/Modules/reserved-names-4.cpp
> b/clang/test/Modules/reserved-names-4.cppm
> similarity index 100%
> rename from clang/test/Modules/reserved-names-4.cpp
> rename to clang/test/Modules/reserved-names-4.cppm
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
