On Thu, Apr 13, 2023 at 9:54 PM chuanqi.xcq <yedeng...@linux.alibaba.com> wrote: > > Hi Aaron, > > 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.
Oh shoot, I thought we did backport that one, but you're right -- it never made it in. Sorry for the noise! ~Aaron > > Thanks, > Chuanqi > > ------------------------------------------------------------------ > From:Aaron Ballman <aa...@aaronballman.com> > Send Time:2023年4月13日(星期四) 20:01 > To:Chuanqi Xu <yedeng...@linux.alibaba.com>; Chuanqi Xu <llvmlist...@llvm.org> > Cc:cfe-commits <cfe-commits@lists.llvm.org> > Subject:Re: [clang] c1f7636 - [C++20] [Modules] Continue parsing after we > found reserved module names > > 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