[clang] c1f7636 - [C++20] [Modules] Continue parsing after we found reserved module names
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 14 04:24:42 PDT 2023
On Thu, Apr 13, 2023 at 9:54 PM chuanqi.xcq <yedeng.yd at 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 <aaron at aaronballman.com>
> Send Time:2023年4月13日(星期四) 20:01
> To:Chuanqi Xu <yedeng.yd at linux.alibaba.com>; Chuanqi Xu <llvmlistbot at llvm.org>
> Cc:cfe-commits <cfe-commits at 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 at 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 at 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 at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list