[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

iains marked an inline comment as done. iains added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:814-815 +diagExportedUnnamedDecl(S, UnnamedDeclKind::Namespace, D, BlockStart); + else +; // We allow an empty named namespace decl. +} else if

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:814-815 +diagExportedUnnamedDecl(S, UnnamedDeclKind::Namespace, D, BlockStart); + else +; // We allow an empty named namespace decl. +} else if

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

This revision was automatically updated to reflect the committed changes. Closed by commit rGf60dc3caa673: [C++20][Modules] Adjust handling of exports of namespaces and using-decls. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

dblaikie accepted this revision. dblaikie added a comment. In D122119#3432220 , @iains wrote: > @dblaikie - is the explanation for the change in diagnostics at the same time > OK? (if not, I am happy to split the patch, but either way I'd like to land

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

iains added a comment. @dblaikie - is the explanation for the change in diagnostics at the same time OK? (if not I can split the patch, but either way I'd like to land what is acceptable soonish). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

iains added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:814-815 +diagExportedUnnamedDecl(S, UnnamedDeclKind::Namespace, D, BlockStart); + else +; // We allow an empty named namespace decl. +} else if

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

iains updated this revision to Diff 418038. iains marked 3 inline comments as done. iains added a comment. rebased, addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122119/new/ https://reviews.llvm.org/D122119 Files:

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

iains added a comment. In D122119#3400267 , @dblaikie wrote: > SOrry, I don't have much context here - the more informative (module/internal > linkage) diagnostic does seem better to me than saying "is not exported", > even if it's a bit esoteric for

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. In D122119#3402168 , @urnathan wrote: > In D122119#3401322 , @ChuanqiXu > wrote: > >> > > > >> I

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

urnathan added a comment. In D122119#3401322 , @ChuanqiXu wrote: > > I think the example is invalid since it violates [[ > http://eel.is/c++draft/module.interface#6 | [module.interface]p6 ]] > explicitly. If this is intended behavior or by design,

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

ChuanqiXu added a comment. In D122119#3400032 , @urnathan wrote: > In D122119#3398949 , @ChuanqiXu > wrote: > >> > > > >> The first feeling I saw the change is that not every C++ programmer knows >> about

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

dblaikie added a comment. SOrry, I don't have much context here - the more informative (module/internal linkage) diagnostic does seem better to me than saying "is not exported", even if it's a bit esoteric for some users. We do have other diagnostics that mention linkage, I'm sure (because

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

urnathan added a comment. In D122119#3398949 , @ChuanqiXu wrote: > > The first feeling I saw the change is that not every C++ programmer knows > about linkage. OK, it depends on the environment really and every one might > has their own opinion.

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

urnathan added a comment. In D122119#3398823 , @iains wrote: > > With the change here we now get: > cannot export redeclaration 'f' here since the previous declaration has > internal linkage > cannot export redeclaration 'S' here since the previous

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

iains marked 2 inline comments as done. iains added a comment. In D122119#3398949 , @ChuanqiXu wrote: > In D122119#3398823 , @iains wrote: > >> So the adjustment to the error message is something I am 50/50 about

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

ChuanqiXu added a comment. In D122119#3398823 , @iains wrote: > So the adjustment to the error message is something I am 50/50 about (IMO it > makes some messages more useful, but maybe not needed in others). > > Without the change we get > "cannot

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

iains marked 2 inline comments as done. iains added subscribers: vsapsai, dblaikie. iains added a comment. So the adjustment to the error message is something I am 50/50 about (IMO it makes some messages more useful, but maybe not needed in others). Without the change we get "cannot export

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

ChuanqiXu added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7832-7833 def err_redeclaration_non_exported : Error < - "cannot export redeclaration %0 here since the previous declaration is not " - "exported">; + "cannot export redeclaration

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

iains created this revision. Herald added a project: All. iains added reviewers: rsmith, urnathan, ChuanqiXu. iains added a subscriber: clang-modules. iains published this revision for review. iains added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. this