<div class="__aliyun_email_body_block"><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">Hi Aaron,</span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;"><br ></span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">   I don't think we need to backport this to 16.x. Since the previous patch https://reviews.llvm.org/D146986 doesn't get backported too. The patch itself is mainly for the implementation of std modules in libcxx. And the std modules in libcxx should target 17.x as far as I know. So it looks not necessary to backport this.</span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;"><br ></span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">Thanks,</span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">Chuanqi</span></div><blockquote  style="margin-right:0;margin-top:0;margin-bottom:0;"><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">------------------------------------------------------------------</span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">From:Aaron Ballman <aaron@aaronballman.com></span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">Send Time:2023年4月13日(星期四) 20:01</span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">To:Chuanqi Xu <yedeng.yd@linux.alibaba.com>; Chuanqi Xu <llvmlistbot@llvm.org></span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">Cc:cfe-commits <cfe-commits@lists.llvm.org></span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">Subject:Re: [clang] c1f7636 - [C++20] [Modules] Continue parsing after we found reserved module names</span></div><div  style="clear:both;"><span  style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;"><br /></span></div>On Thu, Apr 13, 2023 at 3:14 AM Chuanqi Xu via cfe-commits<br ><cfe-commits@lists.llvm.org> wrote:<br >><br >><br >> Author: Chuanqi Xu<br >> Date: 2023-04-13T15:14:34+08:00<br >> New Revision: c1f76363e0db41ab6eb9ebedd687ee098491e9b7<br >><br >> URL: https://github.com/llvm/llvm-project/commit/c1f76363e0db41ab6eb9ebedd687ee098491e9b7<br >> DIFF: https://github.com/llvm/llvm-project/commit/c1f76363e0db41ab6eb9ebedd687ee098491e9b7.diff<br >><br >> LOG: [C++20] [Modules] Continue parsing after we found reserved module names<br >><br >> Close https://github.com/llvm/llvm-project/issues/62112<br >><br >> In the previous change, we'll stop parsing directly after we found<br >> reserved module names. But this may be too aggressive. This patch<br >> changes this. Note that the parsing will still be stopped if the module<br >> name is `module` or `import`.<br ><br >Thank you for fixing this up! I think this should be backported to 16.0.2, WDYT?<br ><br >~Aaron<br ><br >><br >> Added:<br >>     clang/test/Modules/reserved-names-1.cppm<br >>     clang/test/Modules/reserved-names-2.cppm<br >>     clang/test/Modules/reserved-names-3.cppm<br >>     clang/test/Modules/reserved-names-4.cppm<br >><br >> Modified:<br >>     clang/lib/Sema/SemaModule.cpp<br >><br >> Removed:<br >>     clang/test/Modules/reserved-names-1.cpp<br >>     clang/test/Modules/reserved-names-2.cpp<br >>     clang/test/Modules/reserved-names-3.cpp<br >>     clang/test/Modules/reserved-names-4.cpp<br >><br >><br >> ################################################################################<br >> diff  --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp<br >> index 6c39cc0b44ca4..84a1fd854d804 100644<br >> --- a/clang/lib/Sema/SemaModule.cpp<br >> +++ b/clang/lib/Sema/SemaModule.cpp<br >> @@ -162,7 +162,8 @@ static bool DiagReservedModuleName(Sema &S, const IdentifierInfo *II,<br >>    case Invalid:<br >>      return S.Diag(Loc, diag::err_invalid_module_name) << II;<br >>    case Reserved:<br >> -    return S.Diag(Loc, diag::warn_reserved_module_name) << II;<br >> +    S.Diag(Loc, diag::warn_reserved_module_name) << II;<br >> +    return false;<br >>    }<br >>    llvm_unreachable("fell off a fully covered switch");<br >>  }<br >> @@ -267,10 +268,8 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,<br >>    if (!getSourceManager().isInSystemHeader(Path[0].second) &&<br >>        (FirstComponentName == "std" ||<br >>         (FirstComponentName.startswith("std") &&<br >> -        llvm::all_of(FirstComponentName.drop_front(3), &llvm::isDigit)))) {<br >> +        llvm::all_of(FirstComponentName.drop_front(3), &llvm::isDigit))))<br >>      Diag(Path[0].second, diag::warn_reserved_module_name) << Path[0].first;<br >> -    return nullptr;<br >> -  }<br >><br >>    // Then test all of the components in the path to see if any of them are<br >>    // using another kind of reserved or invalid identifier.<br >><br >> diff  --git a/clang/test/Modules/reserved-names-1.cpp b/clang/test/Modules/reserved-names-1.cpp<br >> deleted file mode 100644<br >> index a92c2244f1cb6..0000000000000<br >> --- a/clang/test/Modules/reserved-names-1.cpp<br >> +++ /dev/null<br >> @@ -1,46 +0,0 @@<br >> -// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %s<br >> -// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %s<br >> -<br >> -// expected-note@1 15{{add 'module;' to the start of the file to introduce a global module fragment}}<br >> -<br >> -module std;    // loud-warning {{'std' is a reserved name for a module}}<br >> -module _Test;  // loud-warning {{'_Test' is a reserved name for a module}} \<br >> -                  expected-error {{module declaration must occur at the start of the translation unit}}<br >> -module module; // expected-error {{'module' is an invalid name for a module}} \<br >> -                  expected-error {{module declaration must occur at the start of the translation unit}}<br >> -module std0;   // loud-warning {{'std0' is a reserved name for a module}} \<br >> -                  expected-error {{module declaration must occur at the start of the translation unit}}<br >> -<br >> -export module module; // expected-error {{'module' is an invalid name for a module}} \<br >> -                         expected-error {{module declaration must occur at the start of the translation unit}}<br >> -export module import; // expected-error {{'import' is an invalid name for a module}} \<br >> -                         expected-error {{module declaration must occur at the start of the translation unit}}<br >> -export module _Test;  // loud-warning {{'_Test' is a reserved name for a module}} \<br >> -                         expected-error {{module declaration must occur at the start of the translation unit}}<br >> -export module __test; // loud-warning {{'__test' is a reserved name for a module}} \<br >> -                         expected-error {{module declaration must occur at the start of the translation unit}}<br >> -export module te__st; // loud-warning {{'te__st' is a reserved name for a module}} \<br >> -                         expected-error {{module declaration must occur at the start of the translation unit}}<br >> -export module std;    // loud-warning {{'std' is a reserved name for a module}} \<br >> -                         expected-error {{module declaration must occur at the start of the translation unit}}<br >> -export module std.foo;// loud-warning {{'std' is a reserved name for a module}} \<br >> -                         expected-error {{module declaration must occur at the start of the translation unit}}<br >> -export module std0;   // loud-warning {{'std0' is a reserved name for a module}} \<br >> -                         expected-error {{module declaration must occur at the start of the translation unit}}<br >> -export module std1000000; // loud-warning {{'std1000000' is a reserved name for a module}} \<br >> -                         expected-error {{module declaration must occur at the start of the translation unit}}<br >> -export module should_diag._Test; // loud-warning {{'_Test' is a reserved name for a module}} \<br >> -                                    expected-error {{module declaration must occur at the start of the translation unit}}<br >> -<br >> -// Show that being in a system header doesn't save you from diagnostics about<br >> -// use of an invalid module-name identifier.<br >> -# 34 "reserved-names-1.cpp" 1 3<br >> -export module module;       // expected-error {{'module' is an invalid name for a module}} \<br >> -                               expected-error {{module declaration must occur at the start of the translation unit}}<br >> -<br >> -export module _Test.import; // expected-error {{'import' is an invalid name for a module}} \<br >> -                               expected-error {{module declaration must occur at the start of the translation unit}}<br >> -# 39 "reserved-names-1.cpp" 2 3<br >> -<br >> -// We can still use a reserved name on imoport.<br >> -import std; // expected-error {{module 'std' not found}}<br >><br >> diff  --git a/clang/test/Modules/reserved-names-1.cppm b/clang/test/Modules/reserved-names-1.cppm<br >> new file mode 100644<br >> index 0000000000000..e780f1e35b3b7<br >> --- /dev/null<br >> +++ b/clang/test/Modules/reserved-names-1.cppm<br >> @@ -0,0 +1,154 @@<br >> +// RUN: rm -rf %t<br >> +// RUN: mkdir -p %t<br >> +// RUN: split-file %s %t<br >> +<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/module.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/module.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/import.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/import.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/_Test.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/_Test.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/__test.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/__test.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/te__st.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/te__st.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/std.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.foo.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/std.foo.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std0.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/std0.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std1000000.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/std1000000.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/module.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/module.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/import.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/import.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/_Test.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/_Test.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/__test.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/__test.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/te__st.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/te__st.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/std.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std.foo.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/std.foo.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std0.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/std0.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/std1000000.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/std1000000.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/should_diag._Test.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -DNODIAGNOSTICS -Wno-reserved-module-identifier %t/should_diag._Test.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/system-module.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/system-module.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/system._Test.import.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/system._Test.import.cppm<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,loud %t/user.cpp<br >> +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected -Wno-reserved-module-identifier %t/user.cpp<br >> +<br >> +//--- module.cpp<br >> +module module;  // expected-error {{'module' is an invalid name for a module}}<br >> +<br >> +//--- import.cpp<br >> +module import;  // expected-error {{'import' is an invalid name for a module}}<br >> +<br >> +//--- _Test.cpp<br >> +module _Test;   // loud-warning {{'_Test' is a reserved name for a module}}  \<br >> +                // expected-error {{module '_Test' not found}}<br >> +<br >> +//--- __test.cpp<br >> +module __test; // loud-warning {{'__test' is a reserved name for a module}} \<br >> +               // expected-error {{module '__test' not found}}<br >> +<br >> +//--- te__st.cpp<br >> +module te__st; // loud-warning {{'te__st' is a reserved name for a module}} \<br >> +               // expected-error {{module 'te__st' not found}}<br >> +<br >> +//--- std.cpp<br >> +module std; // loud-warning {{'std' is a reserved name for a module}} \<br >> +            // expected-error {{module 'std' not found}}<br >> +<br >> +//--- std.foo.cpp<br >> +module std.foo; // loud-warning {{'std' is a reserved name for a module}} \<br >> +                // expected-error {{module 'std.foo' not found}}<br >> +<br >> +//--- std0.cpp<br >> +module std0; // loud-warning {{'std0' is a reserved name for a module}} \<br >> +             // expected-error {{module 'std0' not found}}<br >> +<br >> +//--- std1000000.cpp<br >> +module std1000000; // loud-warning {{'std1000000' is a reserved name for a module}} \<br >> +                   // expected-error {{module 'std1000000' not found}}<br >> +<br >> +//--- module.cppm<br >> +export module module; // expected-error {{'module' is an invalid name for a module}}<br >> +<br >> +//--- import.cppm<br >> +export module import; // expected-error {{'import' is an invalid name for a module}}<br >> +<br >> +//--- _Test.cppm<br >> +#ifdef NODIAGNOSTICS<br >> +// expected-no-diagnostics<br >> +#endif<br >> +export module _Test;  // loud-warning {{'_Test' is a reserved name for a module}}<br >> +<br >> +//--- __test.cppm<br >> +#ifdef NODIAGNOSTICS<br >> +// expected-no-diagnostics<br >> +#endif<br >> +export module __test; // loud-warning {{'__test' is a reserved name for a module}}<br >> +export int a = 43;<br >> +<br >> +//--- te__st.cppm<br >> +#ifdef NODIAGNOSTICS<br >> +// expected-no-diagnostics<br >> +#endif<br >> +export module te__st; // loud-warning {{'te__st' is a reserved name for a module}}<br >> +export int a = 43;<br >> +<br >> +//--- std.cppm<br >> +#ifdef NODIAGNOSTICS<br >> +// expected-no-diagnostics<br >> +#endif<br >> +export module std;    // loud-warning {{'std' is a reserved name for a module}}<br >> +<br >> +export int a = 43;<br >> +<br >> +//--- std.foo.cppm<br >> +#ifdef NODIAGNOSTICS<br >> +// expected-no-diagnostics<br >> +#endif<br >> +export module std.foo;// loud-warning {{'std' is a reserved name for a module}}<br >> +<br >> +//--- std0.cppm<br >> +#ifdef NODIAGNOSTICS<br >> +// expected-no-diagnostics<br >> +#endif<br >> +export module std0;   // loud-warning {{'std0' is a reserved name for a module}}<br >> +<br >> +//--- std1000000.cppm<br >> +#ifdef NODIAGNOSTICS<br >> +// expected-no-diagnostics<br >> +#endif<br >> +export module std1000000; // loud-warning {{'std1000000' is a reserved name for a module}}<br >> +<br >> +//--- should_diag._Test.cppm<br >> +#ifdef NODIAGNOSTICS<br >> +// expected-no-diagnostics<br >> +#endif<br >> +export module should_diag._Test; // loud-warning {{'_Test' is a reserved name for a module}}<br >> +<br >> +//--- system-module.cppm<br >> +// Show that being in a system header doesn't save you from diagnostics about<br >> +// use of an invalid module-name identifier.<br >> +# 34 "reserved-names-1.cpp" 1 3<br >> +export module module;       // expected-error {{'module' is an invalid name for a module}}<br >> +<br >> +//--- system._Test.import.cppm<br >> +# 34 "reserved-names-1.cpp" 1 3<br >> +export module _Test.import; // expected-error {{'import' is an invalid name for a module}}<br >> +<br >> +//--- user.cpp<br >> +// We can still use a reserved name on imoport.<br >> +import std; // expected-error {{module 'std' not found}}<br >><br >> diff  --git a/clang/test/Modules/reserved-names-2.cpp b/clang/test/Modules/reserved-names-2.cppm<br >> similarity index 100%<br >> rename from clang/test/Modules/reserved-names-2.cpp<br >> rename to clang/test/Modules/reserved-names-2.cppm<br >><br >> diff  --git a/clang/test/Modules/reserved-names-3.cpp b/clang/test/Modules/reserved-names-3.cppm<br >> similarity index 100%<br >> rename from clang/test/Modules/reserved-names-3.cpp<br >> rename to clang/test/Modules/reserved-names-3.cppm<br >><br >> diff  --git a/clang/test/Modules/reserved-names-4.cpp b/clang/test/Modules/reserved-names-4.cppm<br >> similarity index 100%<br >> rename from clang/test/Modules/reserved-names-4.cpp<br >> rename to clang/test/Modules/reserved-names-4.cppm<br >><br >><br >><br >> _______________________________________________<br >> cfe-commits mailing list<br >> cfe-commits@lists.llvm.org<br >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</blockquote></div>