[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D110215#3178575 , @vitalybuka 
wrote:

>   llvm-project/clang/lib/Sema/SemaModule.cpp:715:70: warning: missing field 
> 'OuterVisibleModules' initializer [-Wmissing-field-initializers]
>   ^

I had tried to fix this in a NFC commit: 4168efe1b 
. Hope 
this works.
(My compiler locally didn't warn for it... I need to update it...)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

  llvm-project/clang/lib/Sema/SemaModule.cpp:715:70: warning: missing field 
'OuterVisibleModules' initializer [-Wmissing-field-initializers]
  ^


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe587372f8510: [C++20] [Module] Support extern C/C++ 
semantics (authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Lex/ModuleMap.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.unit/p7/Inputs/CPP.cppm
  clang/test/CXX/module/module.unit/p7/Inputs/h1.h
  clang/test/CXX/module/module.unit/p7/Inputs/h2.h
  clang/test/CXX/module/module.unit/p7/Inputs/h4.h
  clang/test/CXX/module/module.unit/p7/Inputs/h5.h
  clang/test/CXX/module/module.unit/p7/t1.cpp
  clang/test/CXX/module/module.unit/p7/t2.cpp
  clang/test/CXX/module/module.unit/p7/t3.cpp
  clang/test/CXX/module/module.unit/p7/t4.cpp
  clang/test/CXX/module/module.unit/p7/t5.cpp
  clang/test/CXX/module/module.unit/p7/t6.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.unit/p7/t6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t6.cpp
@@ -0,0 +1,15 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %S/Inputs/CPP.cppm -I%S/Inputs -o %t/X.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %s -verify
+module;
+#include "Inputs/h2.h"
+export module use;
+import X;
+void printX(CPP *cpp) {
+  cpp->print(); // expected-error {{'CPP' must be defined before it is used}}
+// expected-error@-1 {{'CPP' must be defined before it is used}}
+// expected-error@-2 {{no member named 'print' in 'CPP'}}
+// expected-note@Inputs/CPP.cppm:5 {{definition here is not reachable}}
+// expected-note@Inputs/CPP.cppm:5 {{definition here is not reachable}}
+}
Index: clang/test/CXX/module/module.unit/p7/t5.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t5.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;
Index: clang/test/CXX/module/module.unit/p7/t4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.unit/p7/t3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t1.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void 

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 392625.
ChuanqiXu added a comment.

Address comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Lex/ModuleMap.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.unit/p7/Inputs/CPP.cppm
  clang/test/CXX/module/module.unit/p7/Inputs/h1.h
  clang/test/CXX/module/module.unit/p7/Inputs/h2.h
  clang/test/CXX/module/module.unit/p7/Inputs/h4.h
  clang/test/CXX/module/module.unit/p7/Inputs/h5.h
  clang/test/CXX/module/module.unit/p7/t1.cpp
  clang/test/CXX/module/module.unit/p7/t2.cpp
  clang/test/CXX/module/module.unit/p7/t3.cpp
  clang/test/CXX/module/module.unit/p7/t4.cpp
  clang/test/CXX/module/module.unit/p7/t5.cpp
  clang/test/CXX/module/module.unit/p7/t6.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.unit/p7/t6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t6.cpp
@@ -0,0 +1,15 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %S/Inputs/CPP.cppm -I%S/Inputs -o %t/X.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %s -verify
+module;
+#include "Inputs/h2.h"
+export module use;
+import X;
+void printX(CPP *cpp) {
+  cpp->print(); // expected-error {{'CPP' must be defined before it is used}}
+// expected-error@-1 {{'CPP' must be defined before it is used}}
+// expected-error@-2 {{no member named 'print' in 'CPP'}}
+// expected-note@Inputs/CPP.cppm:5 {{definition here is not reachable}}
+// expected-note@Inputs/CPP.cppm:5 {{definition here is not reachable}}
+}
Index: clang/test/CXX/module/module.unit/p7/t5.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t5.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;
Index: clang/test/CXX/module/module.unit/p7/t4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.unit/p7/t3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t1.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+void bar() {
+  return;
+}
+int baz() {
+  return 3;
+}
+double double_func() {
+  return 5.0;
+}
+}
+
+extern "C++" {
+void bar_cpp() {
+  

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks, this looks great. A couple of minor suggestions.




Comment at: clang/include/clang/Lex/ModuleMap.h:542-543
+  /// unit's Module until later, because we don't know what it will be called
+  /// usually. (NOTE: See https://eel.is/c++draft/module.unit#7.2 for the case
+  /// we could know its parent.)
+  Module *createGlobalModuleFragmentForModuleUnit(SourceLocation Loc,

We generally refer to sections of the C++ standard by name not by URL.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16158
+PushGlobalModuleFragment(ExternLoc, /*IsImplicit=*/true);
+D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::Visible);
+D->setLocalOwningModule(GlobalModule);

I think this should be `ModulePrivate` -- such a global module fragment should 
not be visible to importers of this module. (I think it probably doesn't matter 
in practice since visibility is generally monotonically increasing while 
compiling a module unit, but `ModulePrivate` seems more correct in principle.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@aaron.ballman @urnathan @rsmith ping. Might you take a look?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D110215#3139809 , @aaron.ballman 
wrote:

> In D110215#3139589 , @ChuanqiXu 
> wrote:
>
>> Change includes:
>>
>> - Add parent for newly created global module fragment.
>> - Add a test.
>> - Rename test directory. Suddenly I found that the names of the tests in CXX 
>> directory are corresponds to the standard one by one. So my name before is 
>> not good.
>>
>> @aaron.ballman @urnathan It looks like @rsmith is busy now. Could you please 
>> help to review this one? Many Thanks.
>
> @ChuanqiXu -- I'm happy to take a look (all of your modules patches are still 
> on my review queue), but I've been in WG14 meetings all week this week and 
> will be on vacation all week next week. Given the complexity and importance 
> of the patches, it may be a bit before I can give this a thorough review. (If 
> you get approval from other trusted reviewers before I get to anything, 
> please don't let me hold any of the patches up!)

Got it. Many thanks! It is great enough to know the review process is going on. 
I completely understand that we should be very careful about these patches.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D110215#3139589 , @ChuanqiXu wrote:

> Change includes:
>
> - Add parent for newly created global module fragment.
> - Add a test.
> - Rename test directory. Suddenly I found that the names of the tests in CXX 
> directory are corresponds to the standard one by one. So my name before is 
> not good.
>
> @aaron.ballman @urnathan It looks like @rsmith is busy now. Could you please 
> help to review this one? Many Thanks.

@ChuanqiXu -- I'm happy to take a look (all of your modules patches are still 
on my review queue), but I've been in WG14 meetings all week this week and will 
be on vacation all week next week. Given the complexity and importance of the 
patches, it may be a bit before I can give this a thorough review. (If you get 
approval from other trusted reviewers before I get to anything, please don't 
let me hold any of the patches up!)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 388151.
ChuanqiXu added a comment.

Format codes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Lex/ModuleMap.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.unit/p7/Inputs/CPP.cppm
  clang/test/CXX/module/module.unit/p7/Inputs/h1.h
  clang/test/CXX/module/module.unit/p7/Inputs/h2.h
  clang/test/CXX/module/module.unit/p7/Inputs/h4.h
  clang/test/CXX/module/module.unit/p7/Inputs/h5.h
  clang/test/CXX/module/module.unit/p7/t1.cpp
  clang/test/CXX/module/module.unit/p7/t2.cpp
  clang/test/CXX/module/module.unit/p7/t3.cpp
  clang/test/CXX/module/module.unit/p7/t4.cpp
  clang/test/CXX/module/module.unit/p7/t5.cpp
  clang/test/CXX/module/module.unit/p7/t6.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.unit/p7/t6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t6.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %S/Inputs/CPP.cppm -I%S/Inputs -o %t/X.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %s -verify
+module;
+#include "Inputs/h2.h"
+export module use;
+import X;
+void printX(CPP *cpp) {
+  cpp->print(); // expected-error {{'CPP' must be defined before it is used}}
+// expected-note@Inputs/CPP.cppm:5 {{definition here is not reachable}}
+}
Index: clang/test/CXX/module/module.unit/p7/t5.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t5.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;
Index: clang/test/CXX/module/module.unit/p7/t4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.unit/p7/t3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t1.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+void bar() {
+  return;
+}
+int baz() {
+  return 3;
+}
+double double_func() {
+  return 5.0;
+}
+}
+
+extern "C++" {
+void bar_cpp() {
+  return;
+}
+int baz_cpp() {
+  return 3;
+}
+double double_func_cpp() {
+  return 5.0;
+}
+}
Index: clang/test/CXX/module/module.unit/p7/Inputs/h5.h
===
--- /dev/null
+++ 

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 388125.
ChuanqiXu added a comment.

Change includes:

- Add parent for newly created global module fragment.
- Add a test.
- Rename test directory. Suddenly I found that the names of the tests in CXX 
directory are corresponds to the standard one by one. So my name before is not 
good.

@aaron.ballman @urnathan It looks like @rsmith is busy now. Could you please 
help to review this one? Many Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Lex/ModuleMap.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.unit/p7/Inputs/CPP.cppm
  clang/test/CXX/module/module.unit/p7/Inputs/h1.h
  clang/test/CXX/module/module.unit/p7/Inputs/h2.h
  clang/test/CXX/module/module.unit/p7/Inputs/h4.h
  clang/test/CXX/module/module.unit/p7/Inputs/h5.h
  clang/test/CXX/module/module.unit/p7/t1.cpp
  clang/test/CXX/module/module.unit/p7/t2.cpp
  clang/test/CXX/module/module.unit/p7/t3.cpp
  clang/test/CXX/module/module.unit/p7/t4.cpp
  clang/test/CXX/module/module.unit/p7/t5.cpp
  clang/test/CXX/module/module.unit/p7/t6.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.unit/p7/t6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t6.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %S/Inputs/CPP.cppm -I%S/Inputs -o %t/X.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %s -verify
+module;
+#include "Inputs/h2.h"
+export module use;
+import X;
+void printX(CPP *cpp) {
+  cpp->print(); // expected-error {{'CPP' must be defined before it is used}}
+// expected-note@Inputs/CPP.cppm:5 {{definition here is not reachable}}
+}
Index: clang/test/CXX/module/module.unit/p7/t5.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t5.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;
Index: clang/test/CXX/module/module.unit/p7/t4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.unit/p7/t3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t1.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@rsmith ping~


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@rsmith @aaron.ballman gentle ping~


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu marked 7 inline comments as done.
ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:722-723
+void Sema::PopGlobalModuleFragment() {
+  assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
+ "left the wrong module scope, which is not global module fragment");
+  ModuleScopes.pop_back();

aaron.ballman wrote:
> FWIW, this seems to be failing to compile according to the precommit CI.
> ```
> FAILED: tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o
> CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ 
> -DBUILD_EXAMPLES -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG 
> -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> -D__STDC_LIMIT_MACROS -Itools/clang/lib/Sema 
> -I/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema 
> -I/var/lib/buildkite-agent/builds/llvm-project/clang/include 
> -Itools/clang/include -Iinclude 
> -I/var/lib/buildkite-agent/builds/llvm-project/llvm/include -gmlt -fPIC 
> -fvisibility-inlines-hidden -Werror=date-time 
> -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
> -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
> -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
> -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
> -DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT 
> tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -MF 
> tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o.d -o 
> tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -c 
> /var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp
> /var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp:722:53:
>  error: member reference type 'clang::Module *' is a pointer; did you mean to 
> use '->'?
>   assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
>   ~~^
> ->
> /usr/include/assert.h:93:27: note: expanded from macro 'assert'
>  (static_cast  (expr) \
>   ^~~~
> 1 error generated.
> ```
Oh, my bad. I forgot to run tests under debug locally.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 383376.
ChuanqiXu added a comment.

Address comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.linkage_specification/Inputs/h1.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h5.h
  clang/test/CXX/module/module.linkage_specification/p1.cpp
  clang/test/CXX/module/module.linkage_specification/p2.cpp
  clang/test/CXX/module/module.linkage_specification/p3.cpp
  clang/test/CXX/module/module.linkage_specification/p4.cpp
  clang/test/CXX/module/module.linkage_specification/p5.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.linkage_specification/p5.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p5.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;
Index: clang/test/CXX/module/module.linkage_specification/p4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.linkage_specification/p3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p1.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+void bar() {
+  return;
+}
+int baz() {
+  return 3;
+}
+double double_func() {
+  return 5.0;
+}
+}
+
+extern "C++" {
+void bar_cpp() {
+  return;
+}
+int baz_cpp() {
+  return 3;
+}
+double double_func_cpp() {
+  return 5.0;
+}
+}
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h5.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h5.h
@@ -0,0 +1 @@
+extern "C++" int a;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
@@ -0,0 +1 @@
+extern "C" struct C;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
===
--- /dev/null
+++ 

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16155
+  if (getLangOpts().CPlusPlusModules) {
+auto *GlobalModule =
+PushGlobalModuleFragment(ExternLoc, /*IsImplicit=*/true);

`Module *` instead of `auto *` (the type is not spelled out in the 
initialization).



Comment at: clang/lib/Sema/SemaModule.cpp:71
   // module-private (though they do not have module linkage).
-  auto  = PP.getHeaderSearchInfo().getModuleMap();
-  auto *GlobalModule = Map.createGlobalModuleFragmentForModuleUnit(ModuleLoc);
-  assert(GlobalModule && "module creation should not fail");
-
-  // Enter the scope of the global module.
-  ModuleScopes.push_back({});
-  ModuleScopes.back().BeginLoc = ModuleLoc;
-  ModuleScopes.back().Module = GlobalModule;
-  VisibleModules.setVisible(GlobalModule, ModuleLoc);
+  auto *GlobalModule =
+  PushGlobalModuleFragment(ModuleLoc, /*IsImplicit=*/false);

`Module *`



Comment at: clang/lib/Sema/SemaModule.cpp:707-708
+   bool IsImplicit) {
+  auto  = PP.getHeaderSearchInfo().getModuleMap();
+  auto *GlobalModule = Map.createGlobalModuleFragmentForModuleUnit(BeginLoc);
+  assert(GlobalModule && "module creation should not fail");

Please spell these out as well.



Comment at: clang/lib/Sema/SemaModule.cpp:712-715
+  ModuleScopes.push_back({});
+  ModuleScopes.back().BeginLoc = BeginLoc;
+  ModuleScopes.back().Module = GlobalModule;
+  ModuleScopes.back().ImplicitGlobalModuleFragment = IsImplicit;

Can we use `emplace_back()` and construct in place rather than constructing 
piecemeal?



Comment at: clang/lib/Sema/SemaModule.cpp:722-723
+void Sema::PopGlobalModuleFragment() {
+  assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
+ "left the wrong module scope, which is not global module fragment");
+  ModuleScopes.pop_back();

FWIW, this seems to be failing to compile according to the precommit CI.
```
FAILED: tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ 
-DBUILD_EXAMPLES -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG 
-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS -Itools/clang/lib/Sema 
-I/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema 
-I/var/lib/buildkite-agent/builds/llvm-project/clang/include 
-Itools/clang/include -Iinclude 
-I/var/lib/buildkite-agent/builds/llvm-project/llvm/include -gmlt -fPIC 
-fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
-DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT 
tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -MF 
tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o.d -o 
tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -c 
/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp
/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp:722:53:
 error: member reference type 'clang::Module *' is a pointer; did you mean to 
use '->'?
  assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
  ~~^
->
/usr/include/assert.h:93:27: note: expanded from macro 'assert'
 (static_cast  (expr) \
  ^~~~
1 error generated.
```



Comment at: clang/test/CXX/module/module.linkage_specification/p1.cpp:1
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics

Might as well switch all these to use `-std=c++20`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@rsmith gentle ping~


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@rsmith @aaron.ballman gentle ping~


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 379907.
ChuanqiXu added a comment.
Herald added a subscriber: dexonsmith.

Address the comments from @rsmith. It looks much better now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.linkage_specification/Inputs/h1.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h5.h
  clang/test/CXX/module/module.linkage_specification/p1.cpp
  clang/test/CXX/module/module.linkage_specification/p2.cpp
  clang/test/CXX/module/module.linkage_specification/p3.cpp
  clang/test/CXX/module/module.linkage_specification/p4.cpp
  clang/test/CXX/module/module.linkage_specification/p5.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.linkage_specification/p5.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p5.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;
Index: clang/test/CXX/module/module.linkage_specification/p4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.linkage_specification/p3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p1.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+void bar() {
+  return;
+}
+int baz() {
+  return 3;
+}
+double double_func() {
+  return 5.0;
+}
+}
+
+extern "C++" {
+void bar_cpp() {
+  return;
+}
+int baz_cpp() {
+  return 3;
+}
+double double_func_cpp() {
+  return 5.0;
+}
+}
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h5.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h5.h
@@ -0,0 +1 @@
+extern "C++" int a;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
@@ -0,0 +1 @@
+extern "C" struct C;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h2.h

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Instead of moving functions and classes/enums into the global module after 
creating them, it would be better to push a module scope for the global module 
when entering a C or C++ language linkage specification and pop the module 
scope when leaving the linkage specification. There are lots of other kinds of 
declaration that can appear inside a linkage specification (for example, 
variables), and doing it that way would properly handle all of them, not only 
functions and classes.




Comment at: clang/include/clang/Sema/Sema.h:2198
   llvm::SmallVector ModuleScopes;
+  Module *GlobalModule = nullptr;
 

I think this should live on the `ModuleScope` instead of being separate. If we 
process multiple modules in a single compilation, each needs to have its own 
global module fragment so that we can restrict the visibility of the global 
module fragment to the module unit in which it was declared.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 378415.
ChuanqiXu added a comment.

Format.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.linkage_specification/Inputs/h1.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
  clang/test/CXX/module/module.linkage_specification/p1.cpp
  clang/test/CXX/module/module.linkage_specification/p2.cpp
  clang/test/CXX/module/module.linkage_specification/p3.cpp
  clang/test/CXX/module/module.linkage_specification/p4.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.linkage_specification/p4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.linkage_specification/p3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p1.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+extern "C" {
+void bar() {
+  return;
+}
+int baz() {
+  return 3;
+}
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
@@ -0,0 +1 @@
+extern "C" struct C;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
@@ -0,0 +1 @@
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h1.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h1.h
@@ -0,0 +1,6 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -66,10 +66,12 @@
 return nullptr;
   }
 
+  assert(!GlobalModule &&
+ "Global module already created but was not covered above.");
   // We start in the global module; all those declarations are implicitly
   // module-private (though they do not have module linkage).
   auto  = PP.getHeaderSearchInfo().getModuleMap();
-  auto *GlobalModule = 

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:9351
+  (NewFD->isExternCContext() || NewFD->isExternCXXContext())) {
+if (!getGlobalModule())
+  Diag(NewFD->getLocation(),

aaron.ballman wrote:
> I'm a bit confused here. [module.unit]p7 is describing what module a 
> declaration attached to under which circumstances. I don't see a constraint 
> there which should result in a diagnostic. My reading of 
> https://eel.is/c++draft/module.unit#6 is that the global module always 
> exists, so we should be able to attach declarations to it at any point. Am I 
> misunderstanding?
Yeah, after looking into this, I think you are right. We should create the 
global module when we need it but failed to find it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 378401.
ChuanqiXu added a comment.

Address the comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.linkage_specification/Inputs/h1.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
  clang/test/CXX/module/module.linkage_specification/p1.cpp
  clang/test/CXX/module/module.linkage_specification/p2.cpp
  clang/test/CXX/module/module.linkage_specification/p3.cpp
  clang/test/CXX/module/module.linkage_specification/p4.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.linkage_specification/p4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.linkage_specification/p3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p1.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+extern "C" {
+void bar() {
+return;
+}
+int baz() {
+return 3;
+}
+double double_func() {
+return 5.0;
+}
+}
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
@@ -0,0 +1 @@
+extern "C" struct C;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
@@ -0,0 +1 @@
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h1.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h1.h
@@ -0,0 +1,6 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -66,10 +66,13 @@
 return nullptr;
   }
 
+  assert(
+  !GlobalModule &&
+  "Global module already created but was not covered above.");
   // We start in the global module; all those declarations are implicitly
   // module-private (though they 

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: cfe-commits, aaron.ballman.
aaron.ballman added reviewers: aaron.ballman, erichkeane.
aaron.ballman added a comment.

Adding some more reviewers and subscribing the mailing lists.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10842
 def note_prev_module_definition_from_ast_file : Note<"module loaded from 
'%0'">;
+def err_module_langugae_linkage_no_global : Error <
+  "The declaration %0 appears within a linkage-specification should be "

aaron.ballman wrote:
> mizvekov wrote:
> > ```
> > def **err_module_language_linkage_no_global** : Error <
> >   "The declaration %0**, which** appears within a linkage-specification**, 
> > is not "
> >   "attached to **the** global module.">;
> > ```
> > 
> > My two cents since I think the last sentence did not seem to explain the 
> > problem very well.
> Diagnostics in Clang are intentionally ungrammatical, so they should not have 
> leading capitalization or most terminating punctuation. I agree that the 
> diagnostic wording here doesn't really explain what's going wrong.
typo, should be: err_module_language_linkage_no_global 



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10842-10845
+def err_module_langugae_linkage_no_global : Error <
+  "The declaration %0 appears within a linkage-specification should be "
+  "attached to global module. But the compiler failed to search the global "
+  "module.">;

mizvekov wrote:
> ```
> def **err_module_language_linkage_no_global** : Error <
>   "The declaration %0**, which** appears within a linkage-specification**, is 
> not "
>   "attached to **the** global module.">;
> ```
> 
> My two cents since I think the last sentence did not seem to explain the 
> problem very well.
Diagnostics in Clang are intentionally ungrammatical, so they should not have 
leading capitalization or most terminating punctuation. I agree that the 
diagnostic wording here doesn't really explain what's going wrong.



Comment at: clang/lib/Sema/SemaDecl.cpp:9351
+  (NewFD->isExternCContext() || NewFD->isExternCXXContext())) {
+if (!getGlobalModule())
+  Diag(NewFD->getLocation(),

I'm a bit confused here. [module.unit]p7 is describing what module a 
declaration attached to under which circumstances. I don't see a constraint 
there which should result in a diagnostic. My reading of 
https://eel.is/c++draft/module.unit#6 is that the global module always exists, 
so we should be able to attach declarations to it at any point. Am I 
misunderstanding?



Comment at: clang/test/CXX/module/module.linkage_specification/Inputs/h1.h:7
+}
\ No newline at end of file


mizvekov wrote:
> Please if possible make your editor set new lines at EOF.
+1 -- please add a newline to the end of each of these files that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110215/new/

https://reviews.llvm.org/D110215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits