<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 Richard,</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 for reporting this! I found the reason that why it didn't get tested previously is that we didn't use `extern "C"/"C++"` actually since we implemented partitions internally. Sorry for confusing. I would try to work on it.</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><div style="clear:both;"><span style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;"><br ></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:Richard Smith <richard@metafoo.co.uk></span></div><div style="clear:both;"><span style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;">Send Time:2021年12月11日(星期六) 08:28</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] 9791b58 - [C++20 Modules] Don't create global module fragment for extern linkage declaration in GMF already</span></div><div style="clear:both;"><span style="font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;"><br ></span></div><div ><div >On Wed, 8 Dec 2021 at 21:56, Chuanqi Xu via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br ></div><div class="gmail_quote"><br >
Author: Chuanqi Xu<br >
Date: 2021-12-09T13:55:15+08:00<br >
New Revision: 9791b589516b644a6273607b46a9c6661993d667<br ><br >
URL: <a href="https://github.com/llvm/llvm-project/commit/9791b589516b644a6273607b46a9c6661993d667" target="_blank">https://github.com/llvm/llvm-project/commit/9791b589516b644a6273607b46a9c6661993d667</a><br >
DIFF: <a href="https://github.com/llvm/llvm-project/commit/9791b589516b644a6273607b46a9c6661993d667.diff" target="_blank">https://github.com/llvm/llvm-project/commit/9791b589516b644a6273607b46a9c6661993d667.diff</a><br ><br >
LOG: [C++20 Modules] Don't create global module fragment for extern linkage declaration in GMF already<br ><br >
Previously we would create global module fragment for extern linkage<br >
declaration which is alreday in global module fragment. However, it is<br >
clearly redundant to do so. This patch would check if the extern linkage<br >
declaration are already in GMF before we create a GMF for it.<br ><div ><br ></div><div >I'm still seeing some problems even after this patch -- linkage spec declarations within the purview of the module still create new global module fragments, and then the serialization code seems to sometimes get confused because we have multiple submodules with the same name "<global>". Perhaps we should reuse an existing global module fragment if there is one, even when inside the purview of the module?</div><div ><br ></div><div >Here's an example end-to-end testcase for which I'm seeing an assertion failure; I've not got a reduced testcase yet:</div><div ><br ></div><div >$ cat say_hello.cppm<br >module;<br >#include <iostream><br >#include <string_view><br >export module Hello;<br >export void SayHello<br > (std::string_view const &name)<br >{<br > std::cout << "Hello " << name << "!\n";<br >}<br ><br >$ cat hello.cpp<br >#include <string_view><br >import Hello;<br >int main() {<br > SayHello("world");<br > return 0;<br >}<br ></div><div ><br ></div><div >$ ./build/bin/clang++ say_hello.cppm --precompile -o say_hello.pcm -std=c++20</div><div >$ ./build/bin/clang++ say_hello.cppm hello.cpp -fmodule-file=say_hello.pcm -std=c++20</div><div ><br ></div>
Added: <br >
clang/test/CXX/module/module.unit/p7/Inputs/h7.h<br >
clang/test/CXX/module/module.unit/p7/t7.cpp<br ><br >
Modified: <br >
clang/include/clang/Sema/Sema.h<br >
clang/lib/Sema/SemaDeclCXX.cpp<br ><br >
Removed: <br ><br ><br ><br >
################################################################################<br >
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h<br >
index a12dd4db66c13..73c5ad1dd7acf 100644<br >
--- a/clang/include/clang/Sema/Sema.h<br >
+++ b/clang/include/clang/Sema/Sema.h<br >
@@ -2222,6 +2222,12 @@ class Sema final {<br >
return ModuleScopes.empty() ? nullptr : ModuleScopes.back().Module;<br >
}<br ><br >
+ /// Helper function to judge if we are in module purview.<br >
+ /// Return false if we are not in a module.<br >
+ bool isCurrentModulePurview() const {<br >
+ return getCurrentModule() ? getCurrentModule()->isModulePurview() : false;<br >
+ }<br >
+<br >
/// Enter the scope of the global module.<br >
Module *PushGlobalModuleFragment(SourceLocation BeginLoc, bool IsImplicit);<br >
/// Leave the scope of the global module.<br ><br >
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp<br >
index 4a0eda2a700fe..3f6bde1b9ed6a 100644<br >
--- a/clang/lib/Sema/SemaDeclCXX.cpp<br >
+++ b/clang/lib/Sema/SemaDeclCXX.cpp<br >
@@ -16153,7 +16153,10 @@ Decl *Sema::ActOnStartLinkageSpecification(Scope *S, SourceLocation ExternLoc,<br >
/// - ...<br >
/// - appears within a linkage-specification,<br >
/// it is attached to the global module.<br >
- if (getLangOpts().CPlusPlusModules && getCurrentModule()) {<br >
+ ///<br >
+ /// If the declaration is already in global module fragment, we don't<br >
+ /// need to attach it again.<br >
+ if (getLangOpts().CPlusPlusModules && isCurrentModulePurview()) {<br >
Module *GlobalModule =<br >
PushGlobalModuleFragment(ExternLoc, /*IsImplicit=*/true);<br >
D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);<br >
@@ -16177,7 +16180,11 @@ Decl *Sema::ActOnFinishLinkageSpecification(Scope *S,<br >
LSDecl->setRBraceLoc(RBraceLoc);<br >
}<br ><br >
- if (getLangOpts().CPlusPlusModules && getCurrentModule())<br >
+ // If the current module doesn't has Parent, it implies that the<br >
+ // LinkageSpec isn't in the module created by itself. So we don't<br >
+ // need to pop it.<br >
+ if (getLangOpts().CPlusPlusModules && getCurrentModule() &&<br >
+ getCurrentModule()->isGlobalModule() && getCurrentModule()->Parent)<br >
PopGlobalModuleFragment();<br ><br >
PopDeclContext();<br ><br >
diff --git a/clang/test/CXX/module/module.unit/p7/Inputs/h7.h b/clang/test/CXX/module/module.unit/p7/Inputs/h7.h<br >
new file mode 100644<br >
index 0000000000000..cd3b6706d561c<br >
--- /dev/null<br >
+++ b/clang/test/CXX/module/module.unit/p7/Inputs/h7.h<br >
@@ -0,0 +1,10 @@<br >
+#ifndef H7_H<br >
+#define H7_H<br >
+<br >
+extern "C++" {<br >
+class A {};<br >
+}<br >
+<br >
+class B : public A {};<br >
+<br >
+#endif<br ><br >
diff --git a/clang/test/CXX/module/module.unit/p7/t7.cpp b/clang/test/CXX/module/module.unit/p7/t7.cpp<br >
new file mode 100644<br >
index 0000000000000..7ce0cbb964131<br >
--- /dev/null<br >
+++ b/clang/test/CXX/module/module.unit/p7/t7.cpp<br >
@@ -0,0 +1,7 @@<br >
+// RUN: rm -fr %t<br >
+// RUN: mkdir %t<br >
+// RUN: %clang_cc1 -std=c++20 -I%S/Inputs/ %s -verify<br >
+// expected-no-diagnostics<br >
+module;<br >
+#include "h7.h"<br >
+export module X;<br ><br ><br ><br >
_______________________________________________<br >
cfe-commits mailing list<br ><a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br ><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br ></div></div></blockquote><div ><br ></div></div>