from:"Chuanqi Xu via cfe\-commits"

[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)

2024-08-07 Thread Chuanqi Xu via cfe-commits



@@ -848,7 +862,21 @@ ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation 
Loc, Expr *Operand,
   }
 
   auto *RD = Promise->getType()->getAsCXXRecordDecl();
-  auto *Transformed = Operand;
+  bool InplaceCall =
+  isCoroInplaceCall(Operand) &&
+  isAttributedCoroInplaceTask(
+  getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType());
+
+  if (InplaceCall) {
+if (auto *Temporary = dyn_cast(Operand)) {

ChuanqiXu9 wrote:

Maybe it is true, but it still looks better to use `IgnoreImplicit` than the 
pattern match. I had many unhappy experience with the pattern match on AST

https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Fix unused argument warning for `/std:c++20` for precompiled module inputs to `clang-cl` (PR #99300)

2024-08-07 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

/cherry-pick 9d315bc

https://github.com/llvm/llvm-project/pull/99300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Fix unused argument warning for `/std:c++20` for precompiled module inputs to `clang-cl` (PR #99300)

2024-08-07 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 milestoned 
https://github.com/llvm/llvm-project/pull/99300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Fix unused argument warning for `/std:c++20` for precompiled module inputs to `clang-cl` (PR #99300)

2024-08-07 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/99300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Fix unused argument warning for `/std:c++20` for precompiled module inputs to `clang-cl` (PR #99300)

2024-08-07 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/99300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] Don't set modules owner ship information for builtin declarations (PR #102115)

2024-08-07 Thread Chuanqi Xu via cfe-commits



@@ -2376,6 +2376,12 @@ NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, 
unsigned ID,
   FunctionDecl *New = CreateBuiltin(II, R, ID, Loc);
   RegisterLocallyScopedExternCDecl(New, S);
 
+  // Builtin functions shouldn't be owned by any module.
+  if (New->hasOwningModule()) {
+New->setLocalOwningModule(nullptr);

ChuanqiXu9 wrote:

I posted https://github.com/llvm/llvm-project/issues/102290. Let's talk about 
this there. And I think I don't have time to do that in the near future and I 
feel the status quo is not really bad. So I'd like to propose to land this 
patch first and remove this after we make the implementation design better 
someday.

https://github.com/llvm/llvm-project/pull/102115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reland [C++20] [Modules] [Itanium ABI] Generate the vtable in the mod… (PR #102287)

2024-08-07 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/102287
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reland [C++20] [Modules] [Itanium ABI] Generate the vtable in the mod… (PR #102287)

2024-08-07 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/102287

Reland https://github.com/llvm/llvm-project/pull/75912

The differences of this PR between 
https://github.com/llvm/llvm-project/pull/75912 are:

- Fixed a regression in `Decl::isInAnotherModuleUnit()` in DeclBase.cpp pointed 
by @mizvekov and add the corresponding test.
- Fixed the regression in windows 
https://github.com/llvm/llvm-project/issues/97447. The changes are in 
`CodeGenModule::getVTableLinkage` from `clang/lib/CodeGen/CGVTables.cpp`. 
According to the feedbacks from MSVC devs, the linkage of vtables won't 
affected by modules. So I simply skipped the case for MSVC.

Given this is more or less fundamental to the use of modules. I hope we can 
backport this to 19.x. 

>From 7daf703b8d2602bd13d9eb9a9e6c9487205427b0 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 7 Aug 2024 16:05:44 +0800
Subject: [PATCH] Reland [C++20] [Modules] [Itanium ABI] Generate the vtable in
 the module unit of dynamic classes (#75912)

Close https://github.com/llvm/llvm-project/issues/70585 and reflect
https://github.com/itanium-cxx-abi/cxx-abi/issues/170.

The significant change of the patch is: for dynamic classes attached to
module units, we generate the vtable to the attached module units
directly and the key functions for such classes is meaningless.
---
 clang/include/clang/AST/DeclBase.h|   7 ++
 .../include/clang/Serialization/ASTBitCodes.h |   3 +
 clang/include/clang/Serialization/ASTReader.h |   6 +
 clang/include/clang/Serialization/ASTWriter.h |   7 ++
 clang/lib/AST/ASTContext.cpp  |   2 +-
 clang/lib/AST/DeclBase.cpp|  34 +++--
 clang/lib/CodeGen/CGVTables.cpp   |  65 ++
 clang/lib/CodeGen/ItaniumCXXABI.cpp   |   3 +
 clang/lib/Sema/SemaDecl.cpp   |   9 ++
 clang/lib/Sema/SemaDeclCXX.cpp|  14 ++-
 clang/lib/Serialization/ASTReader.cpp |  11 ++
 clang/lib/Serialization/ASTReaderDecl.cpp |   7 ++
 clang/lib/Serialization/ASTWriter.cpp |  33 -
 clang/lib/Serialization/ASTWriterDecl.cpp |   6 +
 clang/test/CodeGenCXX/modules-vtable.cppm |  31 +++--
 clang/test/CodeGenCXX/pr70585.cppm|  47 +++
 clang/test/Modules/pr97313.cppm   | 118 ++
 .../test/Modules/static-func-in-private.cppm  |   8 ++
 clang/test/Modules/vtable-windows.cppm|  26 
 19 files changed, 384 insertions(+), 53 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/pr70585.cppm
 create mode 100644 clang/test/Modules/pr97313.cppm
 create mode 100644 clang/test/Modules/static-func-in-private.cppm
 create mode 100644 clang/test/Modules/vtable-windows.cppm

diff --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 58f0aaba93b71..04dbd1db6cba8 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -670,6 +670,13 @@ class alignas(8) Decl {
   /// Whether this declaration comes from another module unit.
   bool isInAnotherModuleUnit() const;
 
+  /// Whether this declaration comes from the same module unit being compiled.
+  bool isInCurrentModuleUnit() const;
+
+  /// Whether the definition of the declaration should be emitted in external
+  /// sources.
+  bool shouldEmitInExternalSource() const;
+
   /// Whether this declaration comes from explicit global module.
   bool isFromExplicitGlobalModule() const;
 
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index f19eef6c5908d..93d28fbef8e45 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -721,6 +721,9 @@ enum ASTRecordTypes {
 
   /// Record code for \#pragma clang unsafe_buffer_usage begin/end
   PP_UNSAFE_BUFFER_USAGE = 69,
+
+  /// Record code for vtables to emit.
+  VTABLES_TO_EMIT = 70,
 };
 
 /// Record types used within a source manager block.
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 1ae1bf8ec7957..fde39e2c2240e 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -790,6 +790,11 @@ class ASTReader
   /// the consumer eagerly.
   SmallVector EagerlyDeserializedDecls;
 
+  /// The IDs of all vtables to emit. The referenced declarations are passed
+  /// to the consumers's HandleVTable eagerly after passing
+  /// EagerlyDeserializedDecls.
+  SmallVector VTablesToEmit;
+
   /// The IDs of all tentative definitions stored in the chain.
   ///
   /// Sema keeps track of all tentative definitions in a TU because it has to
@@ -1500,6 +1505,7 @@ class ASTReader
   bool isConsumerInterestedIn(Decl *D);
   void PassInterestingDeclsToConsumer();
   void PassInterestingDeclToConsumer(Decl *D);
+  void PassVTableToConsumer(CXXRecordDecl *RD);
 
   void finishPendingActions();
   void dia

[clang] Reland [C++20] [Modules] [Itanium ABI] Generate the vtable in the mod… (PR #102287)

2024-08-07 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 milestoned 
https://github.com/llvm/llvm-project/pull/102287
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)

2024-08-06 Thread Chuanqi Xu via cfe-commits



@@ -8119,6 +8119,35 @@ but do not pass them to the underlying coroutine or pass 
them by value.
 }];
 }
 
+def CoroAwaitElidableDoc : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+The ``[[clang::coro_await_elidable]]`` is a class attribute which can be 
applied
+to a coroutine return type.
+
+When a coroutine function that returns such a type calls another coroutine 
function,
+the compiler performs heap allocation elision when the call to the coroutine 
function

ChuanqiXu9 wrote:

I feel better to explain the heap allocation elision here. I'd like to say:
in this case, the coroutine frame for the callee will be a local variable 
within the enclosing braces in the caller's stack frame. And the local 
variable, like other variables in coroutines, may be collected into the 
coroutine frame, which may be allocated on the heap.

https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)

2024-08-06 Thread Chuanqi Xu via cfe-commits



@@ -848,7 +862,21 @@ ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation 
Loc, Expr *Operand,
   }
 
   auto *RD = Promise->getType()->getAsCXXRecordDecl();
-  auto *Transformed = Operand;
+  bool InplaceCall =
+  isCoroInplaceCall(Operand) &&
+  isAttributedCoroInplaceTask(
+  getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType());
+
+  if (InplaceCall) {
+if (auto *Temporary = dyn_cast(Operand)) {

ChuanqiXu9 wrote:

This is not correct at least it didn't handle `MaterializeTemporaryExpr` and I 
believe there are other patterns. I always don't like such pattern matches in 
AST. 

The ideal solution in my mind is to handle this in parser. But that may be a 
too big change.

Maybe what we can do  here  is:

```
if (auto *Call = dyn_cast(Operand->IgnoreImplicit()))
   Call->setCoroMustElide();
```

https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)

2024-08-06 Thread Chuanqi Xu via cfe-commits



@@ -8119,6 +8119,35 @@ but do not pass them to the underlying coroutine or pass 
them by value.
 }];
 }
 
+def CoroAwaitElidableDoc : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+The ``[[clang::coro_await_elidable]]`` is a class attribute which can be 
applied
+to a coroutine return type.
+
+When a coroutine function that returns such a type calls another coroutine 
function,
+the compiler performs heap allocation elision when the call to the coroutine 
function
+is immediately co_awaited as a prvalue.

ChuanqiXu9 wrote:

I am not sure if the prvalue here is needed. If the call is always immediately 
co_awaited, the value of the call should always be prvalue, right?

https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)

2024-08-06 Thread Chuanqi Xu via cfe-commits



@@ -15,6 +15,7 @@
 
 #include "CoroutineStmtBuilder.h"
 #include "clang/AST/ASTLambda.h"
+#include "clang/AST/ComputeDependence.h"

ChuanqiXu9 wrote:

Is this used?

https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)

2024-08-06 Thread Chuanqi Xu via cfe-commits



@@ -5443,24 +5444,38 @@ RValue CodeGenFunction::EmitRValueForField(LValue LV,
 //======//
 
 RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
- ReturnValueSlot ReturnValue) {
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke) {
+  llvm::CallBase *CallOrInvokeStorage;
+  if (!CallOrInvoke) {
+CallOrInvoke = &CallOrInvokeStorage;
+  }
+
+  auto AddCoroMustElideOnExit = llvm::make_scope_exit([&] {
+if (E->isCoroMustElide()) {
+  auto *I = *CallOrInvoke;
+  if (I)
+I->addFnAttr(llvm::Attribute::CoroMustElide);

ChuanqiXu9 wrote:

It looks like all the changes in CodeGen are served for this. And I am a little 
concerned about if it is worthy to do such a big change for this. 
@efriedma-quic could you take a look here?

And I am wondering if all of the Emit*CallExpr function will be converged to 
one or a few fundamental functions to emit the CallInst like `EmitCall` then we 
can do our job there.

https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)

2024-08-06 Thread Chuanqi Xu via cfe-commits



@@ -8278,4 +8307,3 @@ Declares that a function potentially allocates heap 
memory, and prevents any pot
 of ``nonallocating`` by the compiler.
   }];
 }
-

ChuanqiXu9 wrote:

Unnecessary change

https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] Don't set modules owner ship information for builtin declarations (PR #102115)

2024-08-06 Thread Chuanqi Xu via cfe-commits

@@ -2376,6 +2376,12 @@ NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, 
unsigned ID,
   FunctionDecl *New = CreateBuiltin(II, R, ID, Loc);
   RegisterLocallyScopedExternCDecl(New, S);

+  // Builtin functions shouldn't be owned by any module.
+  if (New->hasOwningModule()) {
+New->setLocalOwningModule(nullptr);

ChuanqiXu9 wrote:

> The bug report is unclear to me.
> 
> Are you saying this is a bug, or are you just saying the AST representation 
> is not clean from a user understanding point of view?

This is a reduced root cause of another problem I saw. 
http://eel.is/c++draft/basic.def.odr#15.3 says the declarations attached to 
named modules are not allowed to be duplicated in different TUs. But currently 
the builtin declarations violate this. And I believe this is only an example 
and we may meet other problems.

> What about builtin TypedefDecls?

This is somewhat coincidence. We would create the builtin TypedefDecls in 
`Sema::Initialize()`. But we haven't assign a module for the TU that time. So 
it works more or less surprisingly. The difference between the builtin typedef 
declaration and the current builtin function decl is that, the builtin typedef 
declaration  are created eagerly and the builtin function decls is created 
lazily.

> Can this situation currently arise for anything else?

IIUC, the above builtin TypedefDecl is an example. 

> It's also strange that they would belong to the TU, and the TU would belong 
> to the module, but the builtin itself would not.

Yes, from the high level, the abstract is more or less not so straight forward. 
Maybe we want to save some typing works initially so we tight the process of 
assigning modules with the process of assigning decl contexts. But I admit, the 
mechanism works pretty well except the few exceptions we raise above.

> If this is a cleanliness issue, can we instead create a builtin module, add 
> all those builtin declarations to a TU belonging to that module, and then 
> implicitly import it in every module?
> Is that idea workable?

First this may not be a cleanliness issue. And I feel the idea is more or less 
not good or match the design. From the perspective of the specification, there 
are only two kinds of module, named module and the unnamed, aka, the global 
module. And **all** declarations should live in modules, either named module 
and the global module. So technically the compiler should assign all the 
declaration not in the named module into the global module. But due to 
historical reasons, we didn't implement the model directly but treat the 
declarations not in a `clang::Module` as if they are in the global module.

So I feel the idea to introduce implicit module may make the model more complex 
and not match the design.

BTW, another idea may be, attach the builtin function decl to the global 
module. But now, the implementation may only create the global module (in 
implementation, it is actually the global module fragment in the specification) 
conditionally, in another word, we will only create the global module fragment 
after we see `module;`. So the idea may be more complex.

So I feel the current one may be the most simple fix.

https://github.com/llvm/llvm-project/pull/102115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] Don't set modules owner ship information for builtin declarations (PR #102115)

2024-08-06 Thread Chuanqi Xu via cfe-commits



@@ -2376,6 +2376,12 @@ NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, 
unsigned ID,
   FunctionDecl *New = CreateBuiltin(II, R, ID, Loc);
   RegisterLocallyScopedExternCDecl(New, S);
 
+  // Builtin functions shouldn't be owned by any module.
+  if (New->hasOwningModule()) {
+New->setLocalOwningModule(nullptr);

ChuanqiXu9 wrote:

> When is the owning module set?

The owning module is set when we new the decl:  
https://github.com/llvm/llvm-project/blob/47bf996abe4fafa8192c78c472d68c6519349e90/clang/lib/AST/DeclBase.cpp#L106-L108.
  So design intention here should be, then we don't need to worry about modules 
ownership at most times when we develope Sema. And it explains the error, when 
we create the Builtin function decl, we set the context as the TU (good), and 
the TU is attached to a named module (good), then we attach the decl to the 
named module (bad).

And if we want to avoid setting the incorrect modules ownership in the first 
place, we need to change the new function. It smells bad. 

I feel the current position is good as long as we can make sure we'll only 
create builtin functions here and it looks true now.

https://github.com/llvm/llvm-project/pull/102115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Fix unused argument warning for `/std:c++20` for precompiled module inputs to `clang-cl` (PR #99300)

2024-08-06 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

Maybe something like this:

```
// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: %clang %t/fake.pcm -std=c++20 -### 2>&1 | FileCheck %t/fake.pcm

//--- fake.pcm
// CHECK-NOT: warning
```

https://github.com/llvm/llvm-project/pull/99300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-08-06 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

/cherry-pick bd576fe34285c4dcd04837bf07a89a9c00e3cd

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-08-06 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 milestoned 
https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-08-06 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

I think this worth to be in 19.x and I don't it is a risk.

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-08-06 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] Don't set modules owner ship information for builtin declarations (PR #102115)

2024-08-06 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/102115

Close https://github.com/llvm/llvm-project/issues/101939

As the issue said, the builtin declarations shouldn't be in any modules.

>From 3dabd3815c78697046ba436e62a2ea39cf2361b3 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Tue, 6 Aug 2024 17:07:01 +0800
Subject: [PATCH] [C++20] [Modules] Don't set modules owner ship information
 for builtin declarations

Close https://github.com/llvm/llvm-project/issues/101939

As the issue said, the builtin declarations shouldn't be in any modules.
---
 clang/lib/Sema/SemaDecl.cpp  | 6 ++
 clang/test/Modules/pr101939.cppm | 6 ++
 2 files changed, 12 insertions(+)
 create mode 100644 clang/test/Modules/pr101939.cppm

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 1f4bfa247b544..a82c66ced817e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2376,6 +2376,12 @@ NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, 
unsigned ID,
   FunctionDecl *New = CreateBuiltin(II, R, ID, Loc);
   RegisterLocallyScopedExternCDecl(New, S);
 
+  // Builtin functions shouldn't be owned by any module.
+  if (New->hasOwningModule()) {
+New->setLocalOwningModule(nullptr);
+New->setModuleOwnershipKind(Decl::ModuleOwnershipKind::Unowned);
+  }
+
   // TUScope is the translation-unit scope to insert this function into.
   // FIXME: This is hideous. We need to teach PushOnScopeChains to
   // relate Scopes to DeclContexts, and probably eliminate CurContext
diff --git a/clang/test/Modules/pr101939.cppm b/clang/test/Modules/pr101939.cppm
new file mode 100644
index 0..35c243ed0f1bf
--- /dev/null
+++ b/clang/test/Modules/pr101939.cppm
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -std=c++20 %s -ast-dump | FileCheck %s
+
+export module mod;
+export auto a = __builtin_expect(true, true);
+
+// CHECK-NOT: FunctionDecl{{.*}} in mod {{.*}} __builtin_expect

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

[clang] [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-08-05 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

@sharadhr it looks like the commit author is not correct. By default it is 
`3754080+shara...@users.noreply.github.com`, but we don't like such fake mail 
account. Please update it with your real mail.

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-08-05 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

It looks like the CI failure is not related. 

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-08-05 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 approved this pull request.

LGTM. I'll merge this after the CI finished.

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1fec981 - [C++20] [Modules] Skip ODR checks in implicit global modules

2024-08-05 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2024-08-05T17:01:24+08:00
New Revision: 1fec981b67ac57abd4d8defd73beb5a9433c602f

URL: 
https://github.com/llvm/llvm-project/commit/1fec981b67ac57abd4d8defd73beb5a9433c602f
DIFF: 
https://github.com/llvm/llvm-project/commit/1fec981b67ac57abd4d8defd73beb5a9433c602f.diff

LOG: [C++20] [Modules] Skip ODR checks in implicit global modules

Previously we skipped the ODR checks in explicit global modules. And due
to similar reasons, we should skip the ODR checks in implicit global
modules too.

Added: 


Modified: 
clang/include/clang/AST/DeclBase.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/AST/DeclBase.cpp
clang/test/Modules/skip-odr-check-in-gmf.cppm

Removed: 




diff  --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 40f01abf384e9..58f0aaba93b71 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -673,6 +673,9 @@ class alignas(8) Decl {
   /// Whether this declaration comes from explicit global module.
   bool isFromExplicitGlobalModule() const;
 
+  /// Whether this declaration comes from global module.
+  bool isFromGlobalModule() const;
+
   /// Whether this declaration comes from a named module.
   bool isInNamedModule() const;
 

diff  --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 76e51ac7ab979..1ae1bf8ec7957 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2478,7 +2478,7 @@ class BitsUnpacker {
 
 inline bool shouldSkipCheckingODR(const Decl *D) {
   return D->getASTContext().getLangOpts().SkipODRCheckInGMF &&
- D->isFromExplicitGlobalModule();
+ D->isFromGlobalModule();
 }
 
 } // namespace clang

diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index a1f70546bde42..98a7746b7d997 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1144,6 +1144,10 @@ bool Decl::isFromExplicitGlobalModule() const {
   return getOwningModule() && getOwningModule()->isExplicitGlobalModule();
 }
 
+bool Decl::isFromGlobalModule() const {
+  return getOwningModule() && getOwningModule()->isGlobalModule();
+}
+
 bool Decl::isInNamedModule() const {
   return getOwningModule() && getOwningModule()->isNamedModule();
 }

diff  --git a/clang/test/Modules/skip-odr-check-in-gmf.cppm 
b/clang/test/Modules/skip-odr-check-in-gmf.cppm
index 3ee7d09224bfa..4a6003287a39a 100644
--- a/clang/test/Modules/skip-odr-check-in-gmf.cppm
+++ b/clang/test/Modules/skip-odr-check-in-gmf.cppm
@@ -35,17 +35,21 @@ module;
 export module a;
 export using ::func;
 
+export extern "C++" bool func1() { return true; }
+
 //--- b.cppm
 module;
 #include "func2.h"
 export module b;
 export using ::func;
 
+export extern "C++" bool func1() { return false; }
+
 //--- test.cc
 import a;
 import b;
 bool test() {
-return func(1, 2);
+return func(1, 2) && func1();
 }
 
 #ifdef IGNORE_ODR_VIOLATION
@@ -53,4 +57,6 @@ bool test() {
 #else
 // expected-error@func2.h:1 {{'func' has 
diff erent definitions in 
diff erent modules;}}
 // expected-note@func1.h:1 {{but in 'a.' found a 
diff erent body}}
+// expected-er...@b.cppm:6 {{'func1' has 
diff erent definitions in 
diff erent modules; definition in module 'b.' first 
diff erence is function body}}
+// expected-n...@a.cppm:6 {{but in 'a.' found a 
diff erent body}}
 #endif



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

[clang] [Modules][Diagnostic] Mention which AST file's options differ from the current TU options. (PR #101413)

2024-08-01 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 approved this pull request.

I did a quick scanning and it looks good except formatting. Please leave a 
chance to @jansvanboda to an another look.

https://github.com/llvm/llvm-project/pull/101413
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Modules][Diagnostic] Mention which AST file's options differ from the current TU options. (PR #101413)

2024-08-01 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/101413
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Modules][Diagnostic] Mention which AST file's options differ from the current TU options. (PR #101413)

2024-08-01 Thread Chuanqi Xu via cfe-commits



@@ -130,7 +130,7 @@ class ASTReaderListener {
   ///
   /// \returns true to indicate the options are invalid or false otherwise.
   virtual bool ReadLanguageOptions(const LangOptions &LangOpts,
-   bool Complain,
+   StringRef Filename, bool Complain,

ChuanqiXu9 wrote:

Let's add a comment to explain the meaning of `Filename`. It is not clear from 
the signature. Or we can rename the parameter to make it claer.

https://github.com/llvm/llvm-project/pull/101413
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Serialization] Use 32 bit aligned decl id instead of unaligned decl id (PR #95348)

2024-08-01 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

Thanks. I just forgot to handle this. Closed.

https://github.com/llvm/llvm-project/pull/95348
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Serialization] Use 32 bit aligned decl id instead of unaligned decl id (PR #95348)

2024-08-01 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/95348
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] Always emit the inline builtins (PR #101278)

2024-07-31 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/101278
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] Always emit the inline builtins (PR #101278)

2024-07-30 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/101278

See the attached test for the motivation example. If we're too greedy to not 
emit the definition for inline builtins, we may meet a middle end crash. And it 
should be good to emit inline builtins always.

>From 7c95608cc09a3d52140edb89dcef8a432dd08a44 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Tue, 30 Jul 2024 17:54:20 +0800
Subject: [PATCH] [C++20] [Modules] Always emit the inline builtins

See the attached test for the motivation example. If we're too greedy to
not emit the definition for inline builtins, we may meet a middle end
crash. And it should be good to emit inline builtins always.
---
 clang/lib/CodeGen/CodeGenModule.cpp | 10 +++
 clang/test/Modules/inline-builtins.cppm | 36 +
 2 files changed, 41 insertions(+), 5 deletions(-)
 create mode 100644 clang/test/Modules/inline-builtins.cppm

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 344a0e538f22a..5a575c535b505 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4022,6 +4022,11 @@ bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
 return true;
 
   const auto *F = cast(GD.getDecl());
+  // Inline builtins declaration must be emitted. They often are fortified
+  // functions.
+  if (F->isInlineBuiltinDeclaration())
+return true;
+
   if (CodeGenOpts.OptimizationLevel == 0 && !F->hasAttr())
 return false;
 
@@ -4067,11 +4072,6 @@ bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
 }
   }
 
-  // Inline builtins declaration must be emitted. They often are fortified
-  // functions.
-  if (F->isInlineBuiltinDeclaration())
-return true;
-
   // PR9614. Avoid cases where the source code is lying to us. An available
   // externally function should have an equivalent function somewhere else,
   // but a function that calls itself through asm label/`__builtin_` trickery 
is
diff --git a/clang/test/Modules/inline-builtins.cppm 
b/clang/test/Modules/inline-builtins.cppm
new file mode 100644
index 0..8a0fffbfc25bc
--- /dev/null
+++ b/clang/test/Modules/inline-builtins.cppm
@@ -0,0 +1,36 @@
+// REQUIRES: !system-windows
+//
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -O3 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -O3 %t/test.cc -fmodule-file=a=%t/a.pcm \
+// RUN:   -emit-llvm -o - | FileCheck %t/test.cc
+
+//--- memmove.h
+typedef long unsigned int size_t;
+extern "C" void *memmove (void *__dest, const void *__src, size_t __n)
+ throw () __attribute__ ((__nonnull__ (1, 2)));
+extern "C" __inline __attribute__ ((__always_inline__)) __attribute__ 
((__gnu_inline__)) void *
+ memmove (void *__dest, const void *__src, size_t __len) throw ()
+{
+  return __builtin_memmove(__dest, __src, __len);
+}
+
+//--- a.cppm
+module;
+#include "memmove.h"
+export module a;
+export using ::memmove;
+
+//--- test.cc
+import a;
+
+void test() {
+  int a, b;
+  unsigned c = 0;
+  memmove(&a, &b, c);
+}
+
+// CHECK-NOT: memmove

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

[clang] ca8a411 - [C++20] [Modules] Always emit the inline builtins

2024-07-30 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2024-07-30T17:56:16+08:00
New Revision: ca8a4111f796fe8533e0af95557875b15becff06

URL: 
https://github.com/llvm/llvm-project/commit/ca8a4111f796fe8533e0af95557875b15becff06
DIFF: 
https://github.com/llvm/llvm-project/commit/ca8a4111f796fe8533e0af95557875b15becff06.diff

LOG: [C++20] [Modules] Always emit the inline builtins

See the attached test for the motivation example. If we're too greedy to
not emit the definition for inline builtins, we may meet a middle end
crash. And it should be good to emit inline builtins always.

Added: 
clang/test/Modules/inline-builtins.cppm

Modified: 
clang/lib/CodeGen/CodeGenModule.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 63ed5b4dd0c31..8057812f80311 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4022,6 +4022,11 @@ bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
 return true;
 
   const auto *F = cast(GD.getDecl());
+  // Inline builtins declaration must be emitted. They often are fortified
+  // functions.
+  if (F->isInlineBuiltinDeclaration())
+return true;
+
   if (CodeGenOpts.OptimizationLevel == 0 && !F->hasAttr())
 return false;
 
@@ -4067,11 +4072,6 @@ bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
 }
   }
 
-  // Inline builtins declaration must be emitted. They often are fortified
-  // functions.
-  if (F->isInlineBuiltinDeclaration())
-return true;
-
   // PR9614. Avoid cases where the source code is lying to us. An available
   // externally function should have an equivalent function somewhere else,
   // but a function that calls itself through asm label/`__builtin_` trickery 
is

diff  --git a/clang/test/Modules/inline-builtins.cppm 
b/clang/test/Modules/inline-builtins.cppm
new file mode 100644
index 0..8f41ae2659513
--- /dev/null
+++ b/clang/test/Modules/inline-builtins.cppm
@@ -0,0 +1,34 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -O3 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -O3 %t/test.cc -fmodule-file=a=%t/a.pcm \
+// RUN:   -emit-llvm -o - | FileCheck %t/test.cc
+
+//--- memmove.h
+typedef long unsigned int size_t;
+extern "C" void *memmove (void *__dest, const void *__src, size_t __n)
+ throw () __attribute__ ((__nonnull__ (1, 2)));
+extern "C" __inline __attribute__ ((__always_inline__)) __attribute__ 
((__gnu_inline__)) void *
+ memmove (void *__dest, const void *__src, size_t __len) throw ()
+{
+  return __builtin_memmove(__dest, __src, __len);
+}
+
+//--- a.cppm
+module;
+#include "memmove.h"
+export module a;
+export using ::memmove;
+
+//--- test.cc
+import a;
+
+void test() {
+  int a, b;
+  unsigned c = 0;
+  memmove(&a, &b, c);
+}
+
+// CHECK-NOT: memmove



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

[clang] [C++20][Modules] Allow using stdarg.h with header units (PR #100739)

2024-07-29 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/100739
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Allow using stdarg.h with header units (PR #100739)

2024-07-29 Thread Chuanqi Xu via cfe-commits



@@ -4543,7 +4544,7 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef 
FileName, ModuleKind Type,
 
   // Mark this identifier as being from an AST file so that we can track
   // whether we need to serialize it.
-  markIdentifierFromAST(*this, *II);
+  markIdentifierFromAST(*this, *II, true);

ChuanqiXu9 wrote:

```suggestion
  markIdentifierFromAST(*this, *II, /*IsModule=*/true);
```

https://github.com/llvm/llvm-project/pull/100739
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Allow using stdarg.h with header units (PR #100739)

2024-07-29 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/100739
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Modules] Fix using `va_list` with modules and a precompiled header. (PR #100837)

2024-07-28 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 approved this pull request.

LGTM.

https://github.com/llvm/llvm-project/pull/100837
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-07-23 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,65 @@
+// REQUIRES: system-windows
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cl /std:c++20 --precompile "%/t/Hello.cppm" "/Fo%/t/Hello.pcm"

ChuanqiXu9 wrote:

Oh, sorry. It turns to be my bad. We should check `-emit-module-interface` for 
`--precompile` instead. I didn't think enough. And since we will check each 
line once with a `CHECK` but the output of `-###` is going to be in one line. 
So it may not be good to have 3 `CHECK` in one file. We can either split them 
by `split-file` or using different prefix for `FileCheck`.

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [clang] Split ObjectFilePCHContainerReader from ObjectFilePCHContainerWriter (PR #99599)

2024-07-23 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/99599
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [clang] Split ObjectFilePCHContainerReader from ObjectFilePCHContainerWriter (PR #99599)

2024-07-23 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/99599

>From 2249d5021fb3f9de213772603893e6fa2a0ff7f8 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Fri, 19 Jul 2024 11:02:36 +0800
Subject: [PATCH] [clang] Split ObjectFilePCHContainerReader from
 ObjectFilePCHContainerWriter

Close https://github.com/llvm/llvm-project/issues/99479

See https://github.com/llvm/llvm-project/issues/99479 for details
---
 ...tions.h => ObjectFilePCHContainerWriter.h} | 10 +---
 .../ObjectFilePCHContainerReader.h| 25 +
 clang/lib/CodeGen/CMakeLists.txt  |  2 +-
 ...s.cpp => ObjectFilePCHContainerWriter.cpp} | 47 +---
 clang/lib/Interpreter/Interpreter.cpp |  3 +-
 clang/lib/Interpreter/InterpreterUtils.h  |  1 -
 clang/lib/Serialization/CMakeLists.txt|  2 +
 .../ObjectFilePCHContainerReader.cpp  | 56 +++
 .../Tooling/DependencyScanning/CMakeLists.txt |  1 -
 .../DependencyScanningWorker.cpp  |  2 +-
 clang/tools/c-index-test/CMakeLists.txt   |  1 -
 clang/tools/c-index-test/core_main.cpp|  2 +-
 clang/tools/clang-check/ClangCheck.cpp|  1 -
 clang/tools/driver/cc1_main.cpp   |  3 +-
 lldb/source/Commands/CommandObjectTarget.cpp  |  2 +-
 lldb/tools/lldb-instr/Instrument.cpp  |  3 +-
 16 files changed, 96 insertions(+), 65 deletions(-)
 rename clang/include/clang/CodeGen/{ObjectFilePCHContainerOperations.h => 
ObjectFilePCHContainerWriter.h} (74%)
 create mode 100644 
clang/include/clang/Serialization/ObjectFilePCHContainerReader.h
 rename clang/lib/CodeGen/{ObjectFilePCHContainerOperations.cpp => 
ObjectFilePCHContainerWriter.cpp} (89%)
 create mode 100644 clang/lib/Serialization/ObjectFilePCHContainerReader.cpp

diff --git a/clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h 
b/clang/include/clang/CodeGen/ObjectFilePCHContainerWriter.h
similarity index 74%
rename from clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h
rename to clang/include/clang/CodeGen/ObjectFilePCHContainerWriter.h
index 7a02d8725885a..26ee9f22258c1 100644
--- a/clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h
+++ b/clang/include/clang/CodeGen/ObjectFilePCHContainerWriter.h
@@ -1,4 +1,4 @@
-//===-- CodeGen/ObjectFilePCHContainerOperations.h - *- C++ 
-*-===//
+//===-- CodeGen/ObjectFilePCHContainerWriter.h --*- C++ 
-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -29,14 +29,6 @@ class ObjectFilePCHContainerWriter : public 
PCHContainerWriter {
   std::shared_ptr Buffer) const 
override;
 };
 
-/// A PCHContainerReader implementation that uses LLVM to
-/// wraps Clang modules inside a COFF, ELF, or Mach-O container.
-class ObjectFilePCHContainerReader : public PCHContainerReader {
-  ArrayRef getFormats() const override;
-
-  /// Returns the serialized AST inside the PCH container Buffer.
-  StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const override;
-};
 }
 
 #endif
diff --git a/clang/include/clang/Serialization/ObjectFilePCHContainerReader.h 
b/clang/include/clang/Serialization/ObjectFilePCHContainerReader.h
new file mode 100644
index 0..6281a3f428869
--- /dev/null
+++ b/clang/include/clang/Serialization/ObjectFilePCHContainerReader.h
@@ -0,0 +1,25 @@
+//===-- Serialization/ObjectFilePCHContainerReader.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_SERIALIZATION_OBJECTFILEPCHCONTAINERREADER_H
+#define LLVM_CLANG_SERIALIZATION_OBJECTFILEPCHCONTAINERREADER_H
+
+#include "clang/Frontend/PCHContainerOperations.h"
+
+namespace clang {
+/// A PCHContainerReader implementation that uses LLVM to
+/// wraps Clang modules inside a COFF, ELF, or Mach-O container.
+class ObjectFilePCHContainerReader : public PCHContainerReader {
+  ArrayRef getFormats() const override;
+
+  /// Returns the serialized AST inside the PCH container Buffer.
+  StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const override;
+};
+} // namespace clang
+
+#endif
diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index 2a179deddcc31..deb7b27266d73 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -110,7 +110,7 @@ add_clang_library(clangCodeGen
   MacroPPCallbacks.cpp
   MicrosoftCXXABI.cpp
   ModuleBuilder.cpp
-  ObjectFilePCHContainerOperations.cpp
+  ObjectFilePCHContainerWriter.cpp
   PatternInit.cpp
   SanitizerMetadata.cpp
   SwiftCallingConv.cpp
diff --git a/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp 
b/clang/lib/Cod

[clang] [Doc] Update documentation for no-transitive-change (PR #96453)

2024-07-18 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/96453
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2f0910d - [C++20] [Modules] Skip ODR checks if either declaration comes from GMF

2024-07-18 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2024-07-19T13:38:57+08:00
New Revision: 2f0910d2d74419ef1ebf814b471af721ee78b464

URL: 
https://github.com/llvm/llvm-project/commit/2f0910d2d74419ef1ebf814b471af721ee78b464
DIFF: 
https://github.com/llvm/llvm-project/commit/2f0910d2d74419ef1ebf814b471af721ee78b464.diff

LOG: [C++20] [Modules] Skip ODR checks if either declaration comes from GMF

This patch tries to workaround the case that:
- in a module unit that imports another module unit
- both the module units including overlapped headers
- the compiler emits false positive ODR violation diagnostics for the
  overlapped headers if ODR check is enabled
- the current module units enables PCH

For the third point, we disabled ODR check if the declarations comes
from GMF. However, due to the forth point, the check whether the
declaration comes from GMF failed. Then we still going to check it and
then the users get false positive checks.

What's worse is that, this always happens in clangd, where will generate
the PCH automatically before parsing the input files.

The root cause of the problem we mixed the modules in the semantical
level and the module in the serialization level.

The problem is pretty fundamental and we need time to fix that. But 19.x
is going to be branched and I hope to give clangd better user
experience. So I decided to land this workaround even if it is pretyy
niche and may only work for the case of clangd's pattern.

Added: 
clang/test/Modules/pch-in-module-units.cppm

Modified: 
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index afdeccaf93a9d..3cb96df12e4da 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9875,6 +9875,7 @@ void ASTReader::finishPendingActions() {
 // We only perform ODR checks for decls not in the explicit
 // global module fragment.
 !shouldSkipCheckingODR(FD) &&
+!shouldSkipCheckingODR(NonConstDefn) &&
 FD->getODRHash() != NonConstDefn->getODRHash()) {
   if (!isa(FD)) {
 PendingFunctionOdrMergeFailures[FD].push_back(NonConstDefn);

diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 3de9d327f1b3b..31ab6c651d59f 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -821,7 +821,7 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) {
   Reader.mergeDefinitionVisibility(OldDef, ED);
   // We don't want to check the ODR hash value for declarations from global
   // module fragment.
-  if (!shouldSkipCheckingODR(ED) &&
+  if (!shouldSkipCheckingODR(ED) && !shouldSkipCheckingODR(OldDef) &&
   OldDef->getODRHash() != ED->getODRHash())
 Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED);
 } else {
@@ -2134,7 +2134,7 @@ void ASTDeclReader::MergeDefinitionData(
   }
 
   // We don't want to check ODR for decls in the global module fragment.
-  if (shouldSkipCheckingODR(MergeDD.Definition))
+  if (shouldSkipCheckingODR(MergeDD.Definition) || shouldSkipCheckingODR(D))
 return;
 
   if (D->getODRHash() != MergeDD.ODRHash) {

diff  --git a/clang/test/Modules/pch-in-module-units.cppm 
b/clang/test/Modules/pch-in-module-units.cppm
new file mode 100644
index 0..790a3af09a0ca
--- /dev/null
+++ b/clang/test/Modules/pch-in-module-units.cppm
@@ -0,0 +1,51 @@
+// Test that we will skip ODR checks for declarations from PCH if they
+// were from GMF.
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/A.cppm \
+// RUN:   -o %t/A.pcm -fskip-odr-check-in-gmf
+// RUN: %clang_cc1 -std=c++20 -DDIFF -x c++-header %t/foo.h \
+// RUN:   -emit-pch -o %t/foo.pch -fskip-odr-check-in-gmf
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -fmodule-file=A=%t/A.pcm -include-pch \
+// RUN:   %t/foo.pch -verify -fsyntax-only -fskip-odr-check-in-gmf
+
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+inline int foo() {
+#ifndef DIFF
+  return 43;
+#else
+  return 45;
+#endif
+}
+
+class f {
+public:
+  int mem() {
+#ifndef DIFF
+return 47;
+#else
+return 45;
+#endif
+  }
+};
+#endif
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+export using ::foo;
+export using ::f;
+
+//--- B.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module B;
+import A;
+export int b = foo() + f().mem();



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

[clang] [lldb] [clang] Split ObjectFilePCHContainerReader from ObjectFilePCHContainerWriter (PR #99599)

2024-07-18 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/99599

Close https://github.com/llvm/llvm-project/issues/99479

See https://github.com/llvm/llvm-project/issues/99479 for details

>From 36e24bd88649e9d5771f1dbb668632d33ffe52d7 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Fri, 19 Jul 2024 11:02:36 +0800
Subject: [PATCH] [clang] Split ObjectFilePCHContainerReader from
 ObjectFilePCHContainerWriter

Close https://github.com/llvm/llvm-project/issues/99479

See https://github.com/llvm/llvm-project/issues/99479 for details
---
 ...tions.h => ObjectFilePCHContainerWriter.h} | 10 +---
 .../ObjectFilePCHContainerReader.h| 26 +
 clang/lib/CodeGen/CMakeLists.txt  |  2 +-
 ...s.cpp => ObjectFilePCHContainerWriter.cpp} | 47 +---
 clang/lib/Interpreter/Interpreter.cpp |  3 +-
 clang/lib/Interpreter/InterpreterUtils.h  |  1 -
 clang/lib/Serialization/CMakeLists.txt|  2 +
 .../ObjectFilePCHContainerReader.cpp  | 56 +++
 .../Tooling/DependencyScanning/CMakeLists.txt |  1 -
 .../DependencyScanningWorker.cpp  |  2 +-
 clang/tools/c-index-test/CMakeLists.txt   |  1 -
 clang/tools/c-index-test/core_main.cpp|  2 +-
 clang/tools/clang-check/ClangCheck.cpp|  1 -
 clang/tools/driver/cc1_main.cpp   |  3 +-
 lldb/source/Commands/CommandObjectTarget.cpp  |  2 +-
 lldb/tools/lldb-instr/Instrument.cpp  |  3 +-
 16 files changed, 97 insertions(+), 65 deletions(-)
 rename clang/include/clang/CodeGen/{ObjectFilePCHContainerOperations.h => 
ObjectFilePCHContainerWriter.h} (73%)
 create mode 100644 
clang/include/clang/Serialization/ObjectFilePCHContainerReader.h
 rename clang/lib/CodeGen/{ObjectFilePCHContainerOperations.cpp => 
ObjectFilePCHContainerWriter.cpp} (89%)
 create mode 100644 clang/lib/Serialization/ObjectFilePCHContainerReader.cpp

diff --git a/clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h 
b/clang/include/clang/CodeGen/ObjectFilePCHContainerWriter.h
similarity index 73%
rename from clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h
rename to clang/include/clang/CodeGen/ObjectFilePCHContainerWriter.h
index 7a02d8725885a..5aef4a2d23de0 100644
--- a/clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h
+++ b/clang/include/clang/CodeGen/ObjectFilePCHContainerWriter.h
@@ -1,4 +1,4 @@
-//===-- CodeGen/ObjectFilePCHContainerOperations.h - *- C++ 
-*-===//
+//===-- CodeGen/ObjectFilePCHContainerWriter.h - *- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -29,14 +29,6 @@ class ObjectFilePCHContainerWriter : public 
PCHContainerWriter {
   std::shared_ptr Buffer) const 
override;
 };
 
-/// A PCHContainerReader implementation that uses LLVM to
-/// wraps Clang modules inside a COFF, ELF, or Mach-O container.
-class ObjectFilePCHContainerReader : public PCHContainerReader {
-  ArrayRef getFormats() const override;
-
-  /// Returns the serialized AST inside the PCH container Buffer.
-  StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const override;
-};
 }
 
 #endif
diff --git a/clang/include/clang/Serialization/ObjectFilePCHContainerReader.h 
b/clang/include/clang/Serialization/ObjectFilePCHContainerReader.h
new file mode 100644
index 0..65b90299545f7
--- /dev/null
+++ b/clang/include/clang/Serialization/ObjectFilePCHContainerReader.h
@@ -0,0 +1,26 @@
+//===-- Serialization/ObjectFilePCHContainerReader.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+
+#ifndef LLVM_CLANG_SERIALIZATION_OBJECTFILEPCHCONTAINERREADER_H
+#define LLVM_CLANG_SERIALIZATION_OBJECTFILEPCHCONTAINERREADER_H
+
+#include "clang/Frontend/PCHContainerOperations.h"
+
+namespace clang {
+/// A PCHContainerReader implementation that uses LLVM to
+/// wraps Clang modules inside a COFF, ELF, or Mach-O container.
+class ObjectFilePCHContainerReader : public PCHContainerReader {
+  ArrayRef getFormats() const override;
+
+  /// Returns the serialized AST inside the PCH container Buffer.
+  StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const override;
+};
+}
+
+#endif
\ No newline at end of file
diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index 2a179deddcc31..deb7b27266d73 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -110,7 +110,7 @@ add_clang_library(clangCodeGen
   MacroPPCallbacks.cpp
   MicrosoftCXXABI.cpp
   ModuleBuilder.cpp
-  ObjectFilePCHContainerOperations.cpp
+  ObjectFilePCHContainerWriter.cpp
   PatternInit.cpp

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

2024-07-18 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 commented:

No more comments from me.

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-18 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-18 Thread Chuanqi Xu via cfe-commits



@@ -161,6 +163,7 @@ clang_target_link_libraries(clangDaemon
   clangAST
   clangASTMatchers
   clangBasic
+  clangDependencyScanning

ChuanqiXu9 wrote:

It looks like it comes from 
https://github.com/llvm/llvm-project/blob/b634e057ddecc41dce046887d0f0854fed305374/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp#L489-L490

but clangd didn't touch this function. So it might be possible to split that. 
I'll file an issue to track this.

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

2024-07-17 Thread Chuanqi Xu via cfe-commits



@@ -3071,6 +3073,45 @@ struct EmbedAnnotationData {
 /// Registry of pragma handlers added by plugins
 using PragmaHandlerRegistry = llvm::Registry;
 
+/// Module/Partition name token sequance.
+///
+/// module-name:
+///   module-name-qualifier[opt] identifier
+///
+/// module-name-qualifier
+///   module-name-qualifier[opt] identifier .
+class ModuleNameInfo {
+  friend class Preprocessor;
+  ArrayRef ModuleName;
+  ArrayRef PartitionName;
+
+  ModuleNameInfo(ArrayRef Module, ArrayRef Partition)
+  : ModuleName(Module), PartitionName(Partition) {}
+
+public:
+  ArrayRef getTokens() const {
+if (ModuleName.empty())
+  return PartitionName;
+if (PartitionName.empty())
+  return ModuleName;
+return ArrayRef(ModuleName.begin(), PartitionName.end());

ChuanqiXu9 wrote:

This makes me slightly surprise. Could you explain why it is valid?

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

2024-07-17 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c184b94 - [C++20] [Modules] Write ODRHash for decls in GMF

2024-07-17 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2024-07-18T11:42:23+08:00
New Revision: c184b94ff6546c8ba8ac54b5127189427567978f

URL: 
https://github.com/llvm/llvm-project/commit/c184b94ff6546c8ba8ac54b5127189427567978f
DIFF: 
https://github.com/llvm/llvm-project/commit/c184b94ff6546c8ba8ac54b5127189427567978f.diff

LOG: [C++20] [Modules] Write ODRHash for decls in GMF

Previously, we skipped calculating ODRHash for decls in GMF when writing
them to .pcm files as an optimization. But actually, it is not
true that this will be a pure optimization. Whether or not it is
beneficial depends on the use cases. For example, if we're writing a
function `a` in module and there are 10 consumers of `a` in other TUs,
then the other TUs will pay for the cost to calculate the ODR hash for
`a` ten times. Then this optimization doesn't work. However, if all the
consumers of the module didn't touch `a`, then we can save the cost to
calculate the ODR hash of `a` for 1 times.

And the assumption to make it was: generally, the consumers of a module
may only consume a small part of the imported module. This is the reason
why we tried to load declarations, types and identifiers lazily. Then it
looks good to do the similar thing for calculating ODR hashs.

It works fine for a long time, until we started to look into the support
of modules in clangd. Then we meet multiple issue reports complaining
we're calculating ODR hash in the wrong place. To workaround these issue
reports, I decided to always write the ODRhash for decls in GMF. In my
local test, I only observed less than 1% compile time regression after
doing this. So it should be fine.

Added: 


Modified: 
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterDecl.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 76032aa836b50..3de9d327f1b3b 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -794,15 +794,12 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) {
   BitsUnpacker EnumDeclBits(Record.readInt());
   ED->setNumPositiveBits(EnumDeclBits.getNextBits(/*Width=*/8));
   ED->setNumNegativeBits(EnumDeclBits.getNextBits(/*Width=*/8));
-  bool ShouldSkipCheckingODR = EnumDeclBits.getNextBit();
   ED->setScoped(EnumDeclBits.getNextBit());
   ED->setScopedUsingClassTag(EnumDeclBits.getNextBit());
   ED->setFixed(EnumDeclBits.getNextBit());
 
-  if (!ShouldSkipCheckingODR) {
-ED->setHasODRHash(true);
-ED->ODRHash = Record.readInt();
-  }
+  ED->setHasODRHash(true);
+  ED->ODRHash = Record.readInt();
 
   // If this is a definition subject to the ODR, and we already have a
   // definition, merge this one into it.
@@ -864,9 +861,6 @@ ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) {
 
 void ASTDeclReader::VisitRecordDecl(RecordDecl *RD) {
   VisitRecordDeclImpl(RD);
-  // We should only reach here if we're in C/Objective-C. There is no
-  // global module fragment.
-  assert(!shouldSkipCheckingODR(RD));
   RD->setODRHash(Record.readInt());
 
   // Maintain the invariant of a redeclaration chain containing only
@@ -1066,7 +1060,6 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
 
   FD->setCachedLinkage((Linkage)FunctionDeclBits.getNextBits(/*Width=*/3));
   FD->setStorageClass((StorageClass)FunctionDeclBits.getNextBits(/*Width=*/3));
-  bool ShouldSkipCheckingODR = FunctionDeclBits.getNextBit();
   FD->setInlineSpecified(FunctionDeclBits.getNextBit());
   FD->setImplicitlyInline(FunctionDeclBits.getNextBit());
   FD->setHasSkippedBody(FunctionDeclBits.getNextBit());
@@ -1096,10 +1089,8 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
   if (FD->isExplicitlyDefaulted())
 FD->setDefaultLoc(readSourceLocation());
 
-  if (!ShouldSkipCheckingODR) {
-FD->ODRHash = Record.readInt();
-FD->setHasODRHash(true);
-  }
+  FD->ODRHash = Record.readInt();
+  FD->setHasODRHash(true);
 
   if (FD->isDefaulted() || FD->isDeletedAsWritten()) {
 // If 'Info' is nonzero, we need to read an DefaultedOrDeletedInfo; if,
@@ -1971,8 +1962,6 @@ void ASTDeclReader::ReadCXXDefinitionData(
 
   BitsUnpacker CXXRecordDeclBits = Record.readInt();
 
-  bool ShouldSkipCheckingODR = CXXRecordDeclBits.getNextBit();
-
 #define FIELD(Name, Width, Merge)  
\
   if (!CXXRecordDeclBits.canGetNextNBits(Width)) \
 CXXRecordDeclBits.updateValue(Record.readInt());   
\
@@ -1981,12 +1970,9 @@ void ASTDeclReader::ReadCXXDefinitionData(
 #include "clang/AST/CXXRecordDeclDefinitionBits.def"
 #undef FIELD
 
-  // We only perform ODR checks for decls not in GMF.
-  if (!ShouldSkipCheckingODR) {
-// Note: the caller has deserialized the IsLambda bit already.
-Data.ODRHash = Record.readInt();
-Data.HasODRHash

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-17 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-17 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

Given this is approved, I'd like to land this to give it more baking times. I 
think the current one is reusable as long as the users can be tolerant about 
the long latency for openning each file. If users want it to be more faster by 
reusing module files, users can test 
https://github.com/ChuanqiXu9/clangd-for-modules. The changes there will be 
contributed to the upstream repo step by step. But the early days testing is 
pretty meaningful. There will be many dark corners can only be found in 
practice.

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-07-17 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,65 @@
+// REQUIRES: system-windows
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cl /std:c++20 --precompile "%/t/Hello.cppm" "/Fo%/t/Hello.pcm"

ChuanqiXu9 wrote:

Maybe something like:

```
// RUN: %clang_cl /std:c++20 --precompile -### -- %s 2>&1 | FileCheck  %s
// CHECK: --precompile
```

and we can do such things for `-fmodule-file=` and `-fprebuilt-module-path=`

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver][clang-cl] Fix unused argument warning for `/std:c++20` for precompiled module inputs to `clang-cl` (PR #99300)

2024-07-17 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 commented:

Please add a test for this too. We can use any file ends with `.pcm` to mimic  
module file in the Driver's test and we can use `CHECK-NOT:` to filter the 
warning.

https://github.com/llvm/llvm-project/pull/99300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits

ChuanqiXu9 wrote:

> thanks a lot, LGTM!
> 
> i think the only big item remaining is injecting resource-dir correctly when 
> running clang-scan-deps.
> 
> the most important follow up (for functionality, rather than optimizations 
> and usability) is indexing the PCMs as we build them (similar to 
> preamble-indexing we run today). For any future travelers, please do start a 
> discussion in advance for that one.

Thanks for reviewing!

For future plans, I still like to chase for optimizations and usability first. 
For example, we need to reuse built module files across opened files. Otherwise 
every time we open a new file, we need to build all the modules dependent. This 
can't be tolerant in real workloads. For example, some modules and their 
dependent modules need 10+ seconds to build. Then it is a nightmare.

For index the PCMs, what's your suggestion and the path to do that? I don't 
have an idea for that yet.

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,335 @@
+//===- ModulesBuilder.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "Compiler.h"
+#include "support/Logger.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+//   {TEMP_DIRS}/clangd/module_files/{hashed-file-name}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database.
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile) {
+  llvm::SmallString<128> HashedPrefix = llvm::sys::path::filename(MainFile);
+  // There might be multiple files with the same name in a project. So 
appending
+  // the hash value of the full path to make sure they won't conflict.
+  HashedPrefix += std::to_string(llvm::hash_value(MainFile));
+
+  llvm::SmallString<256> ResultPattern;
+
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+  llvm::sys::path::append(ResultPattern, "clangd");
+  llvm::sys::path::append(ResultPattern, "module_files");
+
+  llvm::sys::path::append(ResultPattern, HashedPrefix);
+
+  ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+  llvm::SmallString<256> Result;
+  llvm::sys::fs::createUniquePath(ResultPattern, Result,
+  /*MakeAbsolute=*/false);
+
+  llvm::sys::fs::create_directories(Result);
+  return Result;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+  PathRef ModuleFilesPrefix) {
+  llvm::SmallString<256> ModuleFilePath(ModuleFilesPrefix);
+  auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+  llvm::sys::path::append(ModuleFilePath, PrimaryModuleName);
+  if (!PartitionName.empty()) {
+ModuleFilePath.append("-");
+ModuleFilePath.append(PartitionName);
+  }
+
+  ModuleFilePath.append(".pcm");
+  return std::string(ModuleFilePath);
+}
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+  ~FailedPrerequisiteModules() override = default;
+
+  // We shouldn't adjust the compilation commands based on
+  // FailedPrerequisiteModules.
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+  }
+
+  // FailedPrerequisiteModules can never be reused.
+  bool
+  canReuse(const CompilerInvocation &CI,
+   llvm::IntrusiveRefCntPtr) const override {
+return false;
+  }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class StandalonePrerequisiteModules : public PrerequisiteModules {
+public:
+  StandalonePrerequisiteModules() = default;
+
+  StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = 
delete;
+  StandalonePrerequisiteModules
+  operator=(const StandalonePrerequisiteModules &) = delete;
+  StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete;
+  StandalonePrerequisiteModules
+  operator=(StandalonePrerequisiteModules &&) = delete;
+
+  ~StandalonePrerequisiteModules() override = default;
+
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+// Appending all built module files.
+for (auto &RequiredModule : RequiredModules)
+  Options.PrebuiltModuleFiles.insert_or_assign(
+  RequiredModule.ModuleName, RequiredModule.ModuleFilePath);
+  }
+
+  bool canReuse(const CompilerInvocation &CI,
+llvm::IntrusiveRefCntPtr) const 
override;
+
+  bool isModuleUnitBuilt(llvm::StringRef ModuleName) const {
+return BuiltModuleNames.contains(ModuleName);
+  }
+
+  void addModuleFile(llvm::StringRef ModuleName,
+

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,335 @@
+//===- ModulesBuilder.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "Compiler.h"
+#include "support/Logger.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+//   {TEMP_DIRS}/clangd/module_files/{hashed-file-name}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database.
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile) {
+  llvm::SmallString<128> HashedPrefix = llvm::sys::path::filename(MainFile);
+  // There might be multiple files with the same name in a project. So 
appending
+  // the hash value of the full path to make sure they won't conflict.
+  HashedPrefix += std::to_string(llvm::hash_value(MainFile));
+
+  llvm::SmallString<256> ResultPattern;
+
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+  llvm::sys::path::append(ResultPattern, "clangd");
+  llvm::sys::path::append(ResultPattern, "module_files");
+
+  llvm::sys::path::append(ResultPattern, HashedPrefix);
+
+  ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+  llvm::SmallString<256> Result;
+  llvm::sys::fs::createUniquePath(ResultPattern, Result,
+  /*MakeAbsolute=*/false);
+
+  llvm::sys::fs::create_directories(Result);
+  return Result;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+  PathRef ModuleFilesPrefix) {
+  llvm::SmallString<256> ModuleFilePath(ModuleFilesPrefix);
+  auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+  llvm::sys::path::append(ModuleFilePath, PrimaryModuleName);
+  if (!PartitionName.empty()) {
+ModuleFilePath.append("-");
+ModuleFilePath.append(PartitionName);
+  }
+
+  ModuleFilePath.append(".pcm");
+  return std::string(ModuleFilePath);
+}
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+  ~FailedPrerequisiteModules() override = default;
+
+  // We shouldn't adjust the compilation commands based on
+  // FailedPrerequisiteModules.
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+  }
+
+  // FailedPrerequisiteModules can never be reused.
+  bool
+  canReuse(const CompilerInvocation &CI,
+   llvm::IntrusiveRefCntPtr) const override {
+return false;
+  }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class StandalonePrerequisiteModules : public PrerequisiteModules {
+public:
+  StandalonePrerequisiteModules() = default;
+
+  StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = 
delete;
+  StandalonePrerequisiteModules
+  operator=(const StandalonePrerequisiteModules &) = delete;
+  StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete;
+  StandalonePrerequisiteModules
+  operator=(StandalonePrerequisiteModules &&) = delete;
+
+  ~StandalonePrerequisiteModules() override = default;
+
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+// Appending all built module files.
+for (auto &RequiredModule : RequiredModules)
+  Options.PrebuiltModuleFiles.insert_or_assign(
+  RequiredModule.ModuleName, RequiredModule.ModuleFilePath);
+  }
+
+  bool canReuse(const CompilerInvocation &CI,
+llvm::IntrusiveRefCntPtr) const 
override;
+
+  bool isModuleUnitBuilt(llvm::StringRef ModuleName) const {
+return BuiltModuleNames.contains(ModuleName);
+  }
+
+  void addModuleFile(llvm::StringRef ModuleName,
+

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,335 @@
+//===- ModulesBuilder.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "Compiler.h"
+#include "support/Logger.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+//   {TEMP_DIRS}/clangd/module_files/{hashed-file-name}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database.
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile) {
+  llvm::SmallString<128> HashedPrefix = llvm::sys::path::filename(MainFile);
+  // There might be multiple files with the same name in a project. So 
appending
+  // the hash value of the full path to make sure they won't conflict.
+  HashedPrefix += std::to_string(llvm::hash_value(MainFile));
+
+  llvm::SmallString<256> ResultPattern;
+
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+  llvm::sys::path::append(ResultPattern, "clangd");
+  llvm::sys::path::append(ResultPattern, "module_files");
+
+  llvm::sys::path::append(ResultPattern, HashedPrefix);
+
+  ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+  llvm::SmallString<256> Result;
+  llvm::sys::fs::createUniquePath(ResultPattern, Result,
+  /*MakeAbsolute=*/false);
+
+  llvm::sys::fs::create_directories(Result);
+  return Result;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+  PathRef ModuleFilesPrefix) {
+  llvm::SmallString<256> ModuleFilePath(ModuleFilesPrefix);
+  auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+  llvm::sys::path::append(ModuleFilePath, PrimaryModuleName);
+  if (!PartitionName.empty()) {
+ModuleFilePath.append("-");
+ModuleFilePath.append(PartitionName);
+  }
+
+  ModuleFilePath.append(".pcm");
+  return std::string(ModuleFilePath);
+}
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+  ~FailedPrerequisiteModules() override = default;
+
+  // We shouldn't adjust the compilation commands based on
+  // FailedPrerequisiteModules.
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+  }
+
+  // FailedPrerequisiteModules can never be reused.
+  bool
+  canReuse(const CompilerInvocation &CI,
+   llvm::IntrusiveRefCntPtr) const override {
+return false;
+  }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class StandalonePrerequisiteModules : public PrerequisiteModules {
+public:
+  StandalonePrerequisiteModules() = default;
+
+  StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = 
delete;
+  StandalonePrerequisiteModules
+  operator=(const StandalonePrerequisiteModules &) = delete;
+  StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete;
+  StandalonePrerequisiteModules
+  operator=(StandalonePrerequisiteModules &&) = delete;
+
+  ~StandalonePrerequisiteModules() override = default;
+
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+// Appending all built module files.
+for (auto &RequiredModule : RequiredModules)
+  Options.PrebuiltModuleFiles.insert_or_assign(
+  RequiredModule.ModuleName, RequiredModule.ModuleFilePath);
+  }
+
+  bool canReuse(const CompilerInvocation &CI,
+llvm::IntrusiveRefCntPtr) const 
override;
+
+  bool isModuleUnitBuilt(llvm::StringRef ModuleName) const {
+return BuiltModuleNames.contains(ModuleName);
+  }
+
+  void addModuleFile(llvm::StringRef ModuleName,
+

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,335 @@
+//===- ModulesBuilder.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "Compiler.h"
+#include "support/Logger.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+//   {TEMP_DIRS}/clangd/module_files/{hashed-file-name}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database.
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile) {
+  llvm::SmallString<128> HashedPrefix = llvm::sys::path::filename(MainFile);
+  // There might be multiple files with the same name in a project. So 
appending
+  // the hash value of the full path to make sure they won't conflict.
+  HashedPrefix += std::to_string(llvm::hash_value(MainFile));
+
+  llvm::SmallString<256> ResultPattern;
+
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+  llvm::sys::path::append(ResultPattern, "clangd");
+  llvm::sys::path::append(ResultPattern, "module_files");
+
+  llvm::sys::path::append(ResultPattern, HashedPrefix);
+
+  ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+  llvm::SmallString<256> Result;
+  llvm::sys::fs::createUniquePath(ResultPattern, Result,
+  /*MakeAbsolute=*/false);
+
+  llvm::sys::fs::create_directories(Result);
+  return Result;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+  PathRef ModuleFilesPrefix) {
+  llvm::SmallString<256> ModuleFilePath(ModuleFilesPrefix);
+  auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+  llvm::sys::path::append(ModuleFilePath, PrimaryModuleName);
+  if (!PartitionName.empty()) {
+ModuleFilePath.append("-");
+ModuleFilePath.append(PartitionName);
+  }
+
+  ModuleFilePath.append(".pcm");
+  return std::string(ModuleFilePath);
+}
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+  ~FailedPrerequisiteModules() override = default;
+
+  // We shouldn't adjust the compilation commands based on
+  // FailedPrerequisiteModules.
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+  }
+
+  // FailedPrerequisiteModules can never be reused.
+  bool
+  canReuse(const CompilerInvocation &CI,
+   llvm::IntrusiveRefCntPtr) const override {
+return false;
+  }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class StandalonePrerequisiteModules : public PrerequisiteModules {
+public:
+  StandalonePrerequisiteModules() = default;
+
+  StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = 
delete;
+  StandalonePrerequisiteModules
+  operator=(const StandalonePrerequisiteModules &) = delete;
+  StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete;
+  StandalonePrerequisiteModules
+  operator=(StandalonePrerequisiteModules &&) = delete;
+
+  ~StandalonePrerequisiteModules() override = default;
+
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+// Appending all built module files.
+for (auto &RequiredModule : RequiredModules)
+  Options.PrebuiltModuleFiles.insert_or_assign(
+  RequiredModule.ModuleName, RequiredModule.ModuleFilePath);
+  }
+
+  bool canReuse(const CompilerInvocation &CI,
+llvm::IntrusiveRefCntPtr) const 
override;
+
+  bool isModuleUnitBuilt(llvm::StringRef ModuleName) const {
+return BuiltModuleNames.contains(ModuleName);
+  }
+
+  void addModuleFile(llvm::StringRef ModuleName,
+

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,335 @@
+//===- ModulesBuilder.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "Compiler.h"
+#include "support/Logger.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+//   {TEMP_DIRS}/clangd/module_files/{hashed-file-name}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database.
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile) {
+  llvm::SmallString<128> HashedPrefix = llvm::sys::path::filename(MainFile);
+  // There might be multiple files with the same name in a project. So 
appending
+  // the hash value of the full path to make sure they won't conflict.
+  HashedPrefix += std::to_string(llvm::hash_value(MainFile));
+
+  llvm::SmallString<256> ResultPattern;
+
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+  llvm::sys::path::append(ResultPattern, "clangd");
+  llvm::sys::path::append(ResultPattern, "module_files");
+
+  llvm::sys::path::append(ResultPattern, HashedPrefix);
+
+  ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+  llvm::SmallString<256> Result;
+  llvm::sys::fs::createUniquePath(ResultPattern, Result,
+  /*MakeAbsolute=*/false);
+
+  llvm::sys::fs::create_directories(Result);
+  return Result;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+  PathRef ModuleFilesPrefix) {
+  llvm::SmallString<256> ModuleFilePath(ModuleFilesPrefix);
+  auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+  llvm::sys::path::append(ModuleFilePath, PrimaryModuleName);
+  if (!PartitionName.empty()) {
+ModuleFilePath.append("-");
+ModuleFilePath.append(PartitionName);
+  }
+
+  ModuleFilePath.append(".pcm");
+  return std::string(ModuleFilePath);
+}
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+  ~FailedPrerequisiteModules() override = default;
+
+  // We shouldn't adjust the compilation commands based on
+  // FailedPrerequisiteModules.
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+  }
+
+  // FailedPrerequisiteModules can never be reused.
+  bool
+  canReuse(const CompilerInvocation &CI,
+   llvm::IntrusiveRefCntPtr) const override {
+return false;
+  }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class StandalonePrerequisiteModules : public PrerequisiteModules {
+public:
+  StandalonePrerequisiteModules() = default;
+
+  StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = 
delete;
+  StandalonePrerequisiteModules
+  operator=(const StandalonePrerequisiteModules &) = delete;
+  StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete;
+  StandalonePrerequisiteModules
+  operator=(StandalonePrerequisiteModules &&) = delete;
+
+  ~StandalonePrerequisiteModules() override = default;
+
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+// Appending all built module files.
+for (auto &RequiredModule : RequiredModules)
+  Options.PrebuiltModuleFiles.insert_or_assign(
+  RequiredModule.ModuleName, RequiredModule.ModuleFilePath);
+  }
+
+  bool canReuse(const CompilerInvocation &CI,
+llvm::IntrusiveRefCntPtr) const 
override;
+
+  bool isModuleUnitBuilt(llvm::StringRef ModuleName) const {
+return BuiltModuleNames.contains(ModuleName);
+  }
+
+  void addModuleFile(llvm::StringRef ModuleName,
+

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,411 @@
+//===--- PrerequisiteModulesTests.cpp ---*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+/// FIXME: Skip testing on windows temporarily due to the different escaping
+/// code mode.
+#ifndef _WIN32
+
+#include "ModulesBuilder.h"
+#include "ScanningProjectModules.h"
+

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,335 @@
+//===- ModulesBuilder.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "Compiler.h"
+#include "support/Logger.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+//   {TEMP_DIRS}/clangd/module_files/{hashed-file-name}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database.
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile) {
+  llvm::SmallString<128> HashedPrefix = llvm::sys::path::filename(MainFile);
+  // There might be multiple files with the same name in a project. So 
appending
+  // the hash value of the full path to make sure they won't conflict.
+  HashedPrefix += std::to_string(llvm::hash_value(MainFile));
+
+  llvm::SmallString<256> ResultPattern;
+
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+  llvm::sys::path::append(ResultPattern, "clangd");
+  llvm::sys::path::append(ResultPattern, "module_files");
+
+  llvm::sys::path::append(ResultPattern, HashedPrefix);
+
+  ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+  llvm::SmallString<256> Result;
+  llvm::sys::fs::createUniquePath(ResultPattern, Result,
+  /*MakeAbsolute=*/false);
+
+  llvm::sys::fs::create_directories(Result);
+  return Result;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+  PathRef ModuleFilesPrefix) {
+  llvm::SmallString<256> ModuleFilePath(ModuleFilesPrefix);
+  auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+  llvm::sys::path::append(ModuleFilePath, PrimaryModuleName);
+  if (!PartitionName.empty()) {
+ModuleFilePath.append("-");
+ModuleFilePath.append(PartitionName);
+  }
+
+  ModuleFilePath.append(".pcm");
+  return std::string(ModuleFilePath);
+}
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+  ~FailedPrerequisiteModules() override = default;
+
+  // We shouldn't adjust the compilation commands based on
+  // FailedPrerequisiteModules.
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+  }
+
+  // FailedPrerequisiteModules can never be reused.
+  bool
+  canReuse(const CompilerInvocation &CI,
+   llvm::IntrusiveRefCntPtr) const override {
+return false;
+  }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class StandalonePrerequisiteModules : public PrerequisiteModules {
+public:
+  StandalonePrerequisiteModules() = default;
+
+  StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = 
delete;
+  StandalonePrerequisiteModules
+  operator=(const StandalonePrerequisiteModules &) = delete;
+  StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete;
+  StandalonePrerequisiteModules
+  operator=(StandalonePrerequisiteModules &&) = delete;
+
+  ~StandalonePrerequisiteModules() override = default;
+
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+// Appending all built module files.
+for (auto &RequiredModule : RequiredModules)
+  Options.PrebuiltModuleFiles.insert_or_assign(
+  RequiredModule.ModuleName, RequiredModule.ModuleFilePath);
+  }
+
+  bool canReuse(const CompilerInvocation &CI,
+llvm::IntrusiveRefCntPtr) const 
override;
+
+  bool isModuleUnitBuilt(llvm::StringRef ModuleName) const {
+return BuiltModuleNames.contains(ModuleName);
+  }
+
+  void addModuleFile(llvm::StringRef ModuleName,
+

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,199 @@
+//===-- ProjectModules.h -*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProjectModules.h"
+
+#include "support/Logger.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+namespace clang::clangd {
+namespace {
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(vector)` member
+/// function. See the comments of `globalScan` to see the details.
+///
+/// The ModuleDependencyScanner can get the directly required module names for 
a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring the primary module interface for a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that 
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interfere with each other.
+class ModuleDependencyScanner {
+public:
+  ModuleDependencyScanner(
+  std::shared_ptr CDB,
+  const ThreadsafeFS &TFS)
+  : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+  /// The scanned modules dependency information for a specific source file.
+  struct ModuleDependencyInfo {
+/// The name of the module if the file is a module unit.
+std::optional ModuleName;
+/// A list of names for the modules that the file directly depends.
+std::vector RequiredModules;
+  };
+
+  /// Scanning the single file specified by \param FilePath.
+  std::optional scan(PathRef FilePath);
+
+  /// Scanning every source file in the current project to get the
+  ///  to  map.
+  /// TODO: We should find an efficient method to get the 
+  /// to  map. We can make it either by providing
+  /// a global module dependency scanner to monitor every file. Or we
+  /// can simply require the build systems (or even the end users)
+  /// to provide the map.
+  void globalScan();
+
+  /// Get the source file from the module name. Note that the language
+  /// guarantees all the module names are unique in a valid program.
+  /// This function should only be called after globalScan.
+  ///
+  /// TODO: We should handle the case that there are multiple source files
+  /// declaring the same module.
+  PathRef getSourceForModuleName(llvm::StringRef ModuleName) const;
+
+  /// Return the direct required modules. Indirect required modules are not
+  /// included.
+  std::vector getRequiredModules(PathRef File);
+
+private:
+  std::shared_ptr CDB;
+  const ThreadsafeFS &TFS;
+
+  // Whether the scanner has scanned the project globally.
+  bool GlobalScanned = false;
+
+  clang::tooling::dependencies::DependencyScanningService Service;
+
+  // TODO: Add a scanning cache.
+
+  // Map module name to source file path.
+  llvm::StringMap ModuleNameToSource;
+};
+
+std::optional
+ModuleDependencyScanner::scan(PathRef FilePath) {
+  auto Candidates = CDB->getCompileCommands(FilePath);
+  if (Candidates.empty())
+return std::nullopt;
+
+  // Choose the first candidates as the compile commands as the file.
+  // Following the same logic with
+  // DirectoryBasedGlobalCompilationDatabase::getCompileCommand.
+  tooling::CompileCommand Cmd = std::move(Candidates.front());

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -318,6 +309,10 @@ bool StandalonePrerequisiteModules::canReuse(
   Clang.getHeaderSearchOpts().ForceCheckCXX20ModulesInputFiles = true;
   Clang.getHeaderSearchOpts().ValidateASTInputFilesContent = true;
 
+  // Following the practice of clang's driver to suppres the checking for ODR

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [LLVM][Coroutines] Perform HALO on "coro_must_elide" coroutines (PR #98974)

2024-07-16 Thread Chuanqi Xu via cfe-commits

ChuanqiXu9 wrote:

> @ChuanqiXu9
> 
> I was trying to find a stack review solution. See 
> https://reviewstack.dev/llvm/llvm-project/pull/98974
> 
> You can compare between commits on this UI. What's the stacked PRs approach 
> you were talking about?

For example, you have 2 patches A and B, and B dependent on A. Then you can 
send the patch A to `llvm-project`'s branch 
(https://llvm.org/docs/GitHub.html#branches) instead of your repo and open a PR 
to merge the branch-of-A to main, then you can open another patch for B too and 
open a PR to merge the branch-of-B to branch-of-A.

https://github.com/llvm/llvm-project/pull/98974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [LLVM][Coroutines] Perform HALO on "coro_must_elide" coroutines (PR #98974)

2024-07-16 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

It looks like the diff has some problems (it contains clang's change). And how 
do you think about the suggestion to split the middle end patch?

https://github.com/llvm/llvm-project/pull/98974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,65 @@
+// REQUIRES: system-windows
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cl /std:c++20 --precompile "%/t/Hello.cppm" "/Fo%/t/Hello.pcm"

ChuanqiXu9 wrote:

We don't need to do that in the test of Drivers. We can pass `-###` here (You 
can look at other test cases in clang/test/Driver for an example) and check for 
the output. If `-###` is specified, it wouldn't run actually but only print the 
outputs.

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -242,7 +242,7 @@ bool types::isCXX(ID Id) {
   case TY_CXXHUHeader:
   case TY_PP_CXXHeaderUnit:
   case TY_ObjCXXHeader: case TY_PP_ObjCXXHeader:
-  case TY_CXXModule: case TY_PP_CXXModule:
+  case TY_CXXModule: case TY_PP_CXXModule: case TY_ModuleFile:

ChuanqiXu9 wrote:

Fine enough. Please land this as a separate PR.

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -398,6 +398,13 @@ BMIs cannot be shipped in an archive to create a module 
library. Instead, the
 BMIs(``*.pcm``) are compiled into object files(``*.o``) and those object files
 are added to the archive instead.
 
+clang-cl
+
+
+``clang-cl`` supports the same options as ``clang++`` for modules as detailed 
above;
+there is no need to prefix these options with ``/clang:``. Note that ``cl.exe``
+options to emit `IFC files 
`
 are *not* supported. The precompiled modules are also not compatible for use 
with ``cl.exe``.

ChuanqiXu9 wrote:

I'd like to add a statement to give suggestion for build systems:

It may be something like (You can reword it)

```
The build systems are suggested to use the option here for `clang-cl` instead 
of `cl.exe`.
```

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -1176,12 +1183,12 @@ have ``.cppm`` (or ``.ccm``, ``.cxxm``, ``.c++m``) as 
the file extension.
 However, the behavior is inconsistent with other compilers. This is tracked by
 `#57416 `_.
 
-clang-cl is not compatible with standard C++ modules
-
+.. clang-cl is not compatible with standard C++ modules

ChuanqiXu9 wrote:

I think we can remove this paragraph directly.

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-07-16 Thread Chuanqi Xu via cfe-commits



@@ -242,7 +242,7 @@ bool types::isCXX(ID Id) {
   case TY_CXXHUHeader:
   case TY_PP_CXXHeaderUnit:
   case TY_ObjCXXHeader: case TY_PP_ObjCXXHeader:
-  case TY_CXXModule: case TY_PP_CXXModule:
+  case TY_CXXModule: case TY_PP_CXXModule: case TY_ModuleFile:

ChuanqiXu9 wrote:

If this is needed, it should be in a separate patch. Let's do one thing in one 
PR.

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_inplace_task]] (PR #98971)

2024-07-16 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

BTW, it will be better to use stacked PR for this one and 
https://github.com/llvm/llvm-project/pull/98974. 

https://github.com/llvm/llvm-project/pull/98971
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_inplace_task]] (PR #98971)

2024-07-16 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

Haven't look into the details but just a quick scanning.

I feel the name `[[coro_inplace_task]]` is not so good. Since it doesn't happen 
unconditionally. It happens conditionally. I still want to try to make it clear 
in the name. I still like names  like `[[coro_elide_after_await]]`, which I 
feel is more clear.

Another point is, I feel the summary may be hard for other people or future 
contributors to understand. I feel it may be better to split it to 3 parts: one 
for C++'s end users (or you can skip this part in the summary since this should 
be present in Attr.doc), a high level introduction to clang's developers for 
what this PR is for, then an explanation what this patch did in low level for 
reviewers of the patch. (So the reviewers don't need to dig this out by 
themselves.)

https://github.com/llvm/llvm-project/pull/98971
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-16 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

@kadircet  Comments addressed. Maybe due to some github's issue, my replies are 
separated and didn't form a standalone list. Hope this won't make it harder.

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -51,6 +52,9 @@
 
 namespace clang {
 namespace clangd {
+
+extern llvm::cl::opt ExperimentalModulesSupport;

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,355 @@
+//===--- PrerequisiteModulesTests.cpp ---*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+/// FIXME: Skip testing on windows temporarily due to the different escaping
+/// code mode.
+#ifndef _WIN32
+
+#include "ModulesBuilder.h"
+#include "ScanningProjectModules.h"
+
+#include "Annotations.h"
+#include "CodeComplete.h"
+#include "Compiler.h"
+#include "TestTU.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang::clangd {
+namespace {
+
+class MockDirectoryCompilationDatabase : public MockCompilationDatabase {
+public:
+  MockDirectoryCompilationDatabase(StringRef TestDir, const ThreadsafeFS &TFS)
+  : MockCompilationDatabase(TestDir),
+MockedCDBPtr(std::make_shared(*this)),
+TFS(TFS) {
+this->ExtraClangFlags.push_back("-std=c++20");
+this->ExtraClangFlags.push_back("-c");
+  }
+
+  void addFile(llvm::StringRef Path, llvm::StringRef Contents);
+
+  std::unique_ptr getProjectModules(PathRef) const override {
+return scanningProjectModules(MockedCDBPtr, TFS);
+  }
+
+private:
+  class MockClangCompilationDatabase : public tooling::CompilationDatabase {
+  public:
+MockClangCompilationDatabase(MockDirectoryCompilationDatabase &MCDB)
+: MCDB(MCDB) {}
+
+std::vector
+getCompileCommands(StringRef FilePath) const override {
+  std::optional Cmd =
+  MCDB.getCompileCommand(FilePath);
+  EXPECT_TRUE(Cmd);
+  return {*Cmd};
+}
+
+std::vector getAllFiles() const override { return Files; }
+
+void AddFile(StringRef File) { Files.push_back(File.str()); }
+
+  private:
+MockDirectoryCompilationDatabase &MCDB;
+std::vector Files;
+  };
+
+  std::shared_ptr MockedCDBPtr;
+  const ThreadsafeFS &TFS;
+};
+
+// Add files to the working testing directory and the compilation database.
+void MockDirectoryCompilationDatabase::addFile(llvm::StringRef Path,
+   llvm::StringRef Contents) {
+  ASSERT_FALSE(llvm::sys::path::is_absolute(Path));
+
+  SmallString<256> AbsPath(Directory);
+  llvm::sys::path::append(AbsPath, Path);
+
+  ASSERT_FALSE(
+  
llvm::sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath)));
+
+  std::error_code EC;
+  llvm::raw_fd_ostream OS(AbsPath, EC);
+  ASSERT_FALSE(EC);
+  OS << Contents;
+
+  MockedCDBPtr->AddFile(Path);
+}
+
+class PrerequisiteModulesTests : public ::testing::Test {
+protected:
+  void SetUp() override {
+ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("modules-test", 
TestDir));
+  }
+
+  void TearDown() override {
+ASSERT_FALSE(llvm::sys::fs::remove_directories(TestDir));
+  }
+
+public:
+  // Get the absolute path for file specified by Path under testing working
+  // directory.
+  std::string getFullPath(llvm::StringRef Path) {
+SmallString<128> Result(TestDir);
+llvm::sys::path::append(Result, Path);
+EXPECT_TRUE(llvm::sys::fs::exists(Result.str()));
+return Result.str().str();
+  }
+
+  std::unique_ptr getGlobalCompilationDatabase() {
+// The compilation flags with modules are much complex so it looks better
+// to use DirectoryBasedGlobalCompilationDatabase than a mocked compilation
+// database.
+DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS);
+return std::make_unique(Opts);
+  }
+
+  ParseInputs getInputs(llvm::StringRef FileName,
+const GlobalCompilationDatabase &CDB) {
+std::string FullPathName = getFullPath(FileName);
+
+ParseInputs Inputs;
+std::optional Cmd =
+CDB.getCompileCommand(FullPathName);
+EXPECT_TRUE(Cmd);
+Inputs.CompileCommand = std::move(*Cmd);
+Inputs.TFS = &TFS;
+
+if (auto Contents = TFS.view(TestDir)->getBufferForFile(FullPathName))
+  Inputs.Contents = Contents->get()->getBuffer().str();
+
+return Inputs;
+  }
+
+  SmallString<256> TestDir;
+  // Noticed MockFS but its member variable 'OverlayRealFileSystemForModules'
+  // implies that it will better to use RealThreadsafeFS directly.
+  RealThreadsafeFS TFS;
+
+  DiagnosticConsumer DiagConsumer;
+};
+
+TEST_F(PrerequisiteModulesTests, PrerequisiteModulesTest) {
+  MockDirectoryCompilationDatabase CDB(TestDir, TFS);
+
+  CDB.addFile("foo.h", R"cpp(
+inline void foo() {}
+  )cpp");
+
+  CDB.addFile("M.cppm", R"cpp(
+module;
+#include "foo.h"
+export module M;
+  )cpp");
+
+  CDB.addFile("N.cppm", R"cpp(
+export module N;
+import :Part;
+import M;
+  )cpp");
+
+  CDB.addFile("N-part.cppm", R"cpp(
+// Different name with filename intentionally.
+export module N:Part;
+  )cpp");

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,355 @@
+//===--- PrerequisiteModulesTests.cpp ---*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+/// FIXME: Skip testing on windows temporarily due to the different escaping
+/// code mode.
+#ifndef _WIN32
+
+#include "ModulesBuilder.h"
+#include "ScanningProjectModules.h"
+
+#include "Annotations.h"
+#include "CodeComplete.h"
+#include "Compiler.h"
+#include "TestTU.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang::clangd {
+namespace {
+
+class MockDirectoryCompilationDatabase : public MockCompilationDatabase {
+public:
+  MockDirectoryCompilationDatabase(StringRef TestDir, const ThreadsafeFS &TFS)
+  : MockCompilationDatabase(TestDir),
+MockedCDBPtr(std::make_shared(*this)),
+TFS(TFS) {
+this->ExtraClangFlags.push_back("-std=c++20");
+this->ExtraClangFlags.push_back("-c");
+  }
+
+  void addFile(llvm::StringRef Path, llvm::StringRef Contents);
+
+  std::unique_ptr getProjectModules(PathRef) const override {
+return scanningProjectModules(MockedCDBPtr, TFS);
+  }
+
+private:
+  class MockClangCompilationDatabase : public tooling::CompilationDatabase {
+  public:
+MockClangCompilationDatabase(MockDirectoryCompilationDatabase &MCDB)
+: MCDB(MCDB) {}
+
+std::vector
+getCompileCommands(StringRef FilePath) const override {
+  std::optional Cmd =
+  MCDB.getCompileCommand(FilePath);
+  EXPECT_TRUE(Cmd);
+  return {*Cmd};
+}
+
+std::vector getAllFiles() const override { return Files; }
+
+void AddFile(StringRef File) { Files.push_back(File.str()); }
+
+  private:
+MockDirectoryCompilationDatabase &MCDB;
+std::vector Files;
+  };
+
+  std::shared_ptr MockedCDBPtr;
+  const ThreadsafeFS &TFS;
+};
+
+// Add files to the working testing directory and the compilation database.
+void MockDirectoryCompilationDatabase::addFile(llvm::StringRef Path,
+   llvm::StringRef Contents) {
+  ASSERT_FALSE(llvm::sys::path::is_absolute(Path));
+
+  SmallString<256> AbsPath(Directory);
+  llvm::sys::path::append(AbsPath, Path);
+
+  ASSERT_FALSE(
+  
llvm::sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath)));
+
+  std::error_code EC;
+  llvm::raw_fd_ostream OS(AbsPath, EC);
+  ASSERT_FALSE(EC);
+  OS << Contents;
+
+  MockedCDBPtr->AddFile(Path);
+}
+
+class PrerequisiteModulesTests : public ::testing::Test {
+protected:
+  void SetUp() override {
+ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("modules-test", 
TestDir));
+  }
+
+  void TearDown() override {
+ASSERT_FALSE(llvm::sys::fs::remove_directories(TestDir));
+  }
+
+public:
+  // Get the absolute path for file specified by Path under testing working
+  // directory.
+  std::string getFullPath(llvm::StringRef Path) {
+SmallString<128> Result(TestDir);
+llvm::sys::path::append(Result, Path);
+EXPECT_TRUE(llvm::sys::fs::exists(Result.str()));
+return Result.str().str();
+  }
+
+  std::unique_ptr getGlobalCompilationDatabase() {
+// The compilation flags with modules are much complex so it looks better
+// to use DirectoryBasedGlobalCompilationDatabase than a mocked compilation
+// database.
+DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS);
+return std::make_unique(Opts);
+  }
+
+  ParseInputs getInputs(llvm::StringRef FileName,
+const GlobalCompilationDatabase &CDB) {
+std::string FullPathName = getFullPath(FileName);
+
+ParseInputs Inputs;
+std::optional Cmd =
+CDB.getCompileCommand(FullPathName);
+EXPECT_TRUE(Cmd);
+Inputs.CompileCommand = std::move(*Cmd);
+Inputs.TFS = &TFS;
+
+if (auto Contents = TFS.view(TestDir)->getBufferForFile(FullPathName))
+  Inputs.Contents = Contents->get()->getBuffer().str();
+
+return Inputs;
+  }
+
+  SmallString<256> TestDir;
+  // Noticed MockFS but its member variable 'OverlayRealFileSystemForModules'
+  // implies that it will better to use RealThreadsafeFS directly.
+  RealThreadsafeFS TFS;
+
+  DiagnosticConsumer DiagConsumer;
+};
+
+TEST_F(PrerequisiteModulesTests, PrerequisiteModulesTest) {
+  MockDirectoryCompilationDatabase CDB(TestDir, TFS);
+
+  CDB.addFile("foo.h", R"cpp(
+inline void foo() {}
+  )cpp");
+
+  CDB.addFile("M.cppm", R"cpp(
+module;
+#include "foo.h"
+export module M;
+  )cpp");
+
+  CDB.addFile("N.cppm", R"cpp(
+export module N;
+import :Part;
+import M;
+  )cpp");
+
+  CDB.addFile("N-part.cppm", R"cpp(
+// Different name with filename intentionally.
+export module N:Part;
+  )cpp");

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,355 @@
+//===--- PrerequisiteModulesTests.cpp ---*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+/// FIXME: Skip testing on windows temporarily due to the different escaping
+/// code mode.
+#ifndef _WIN32
+
+#include "ModulesBuilder.h"
+#include "ScanningProjectModules.h"
+
+#include "Annotations.h"
+#include "CodeComplete.h"
+#include "Compiler.h"
+#include "TestTU.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang::clangd {
+namespace {
+
+class MockDirectoryCompilationDatabase : public MockCompilationDatabase {
+public:
+  MockDirectoryCompilationDatabase(StringRef TestDir, const ThreadsafeFS &TFS)
+  : MockCompilationDatabase(TestDir),
+MockedCDBPtr(std::make_shared(*this)),
+TFS(TFS) {
+this->ExtraClangFlags.push_back("-std=c++20");
+this->ExtraClangFlags.push_back("-c");
+  }
+
+  void addFile(llvm::StringRef Path, llvm::StringRef Contents);
+
+  std::unique_ptr getProjectModules(PathRef) const override {
+return scanningProjectModules(MockedCDBPtr, TFS);
+  }
+
+private:
+  class MockClangCompilationDatabase : public tooling::CompilationDatabase {
+  public:
+MockClangCompilationDatabase(MockDirectoryCompilationDatabase &MCDB)
+: MCDB(MCDB) {}
+
+std::vector
+getCompileCommands(StringRef FilePath) const override {
+  std::optional Cmd =
+  MCDB.getCompileCommand(FilePath);
+  EXPECT_TRUE(Cmd);
+  return {*Cmd};
+}
+
+std::vector getAllFiles() const override { return Files; }
+
+void AddFile(StringRef File) { Files.push_back(File.str()); }
+
+  private:
+MockDirectoryCompilationDatabase &MCDB;
+std::vector Files;
+  };
+
+  std::shared_ptr MockedCDBPtr;
+  const ThreadsafeFS &TFS;
+};
+
+// Add files to the working testing directory and the compilation database.
+void MockDirectoryCompilationDatabase::addFile(llvm::StringRef Path,
+   llvm::StringRef Contents) {
+  ASSERT_FALSE(llvm::sys::path::is_absolute(Path));
+
+  SmallString<256> AbsPath(Directory);
+  llvm::sys::path::append(AbsPath, Path);
+
+  ASSERT_FALSE(
+  
llvm::sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath)));
+
+  std::error_code EC;
+  llvm::raw_fd_ostream OS(AbsPath, EC);
+  ASSERT_FALSE(EC);
+  OS << Contents;
+
+  MockedCDBPtr->AddFile(Path);
+}
+
+class PrerequisiteModulesTests : public ::testing::Test {
+protected:
+  void SetUp() override {
+ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("modules-test", 
TestDir));
+  }
+
+  void TearDown() override {
+ASSERT_FALSE(llvm::sys::fs::remove_directories(TestDir));
+  }
+
+public:
+  // Get the absolute path for file specified by Path under testing working
+  // directory.
+  std::string getFullPath(llvm::StringRef Path) {
+SmallString<128> Result(TestDir);
+llvm::sys::path::append(Result, Path);
+EXPECT_TRUE(llvm::sys::fs::exists(Result.str()));
+return Result.str().str();
+  }
+
+  std::unique_ptr getGlobalCompilationDatabase() {
+// The compilation flags with modules are much complex so it looks better
+// to use DirectoryBasedGlobalCompilationDatabase than a mocked compilation
+// database.
+DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS);
+return std::make_unique(Opts);
+  }
+
+  ParseInputs getInputs(llvm::StringRef FileName,
+const GlobalCompilationDatabase &CDB) {
+std::string FullPathName = getFullPath(FileName);
+
+ParseInputs Inputs;
+std::optional Cmd =
+CDB.getCompileCommand(FullPathName);
+EXPECT_TRUE(Cmd);
+Inputs.CompileCommand = std::move(*Cmd);
+Inputs.TFS = &TFS;
+
+if (auto Contents = TFS.view(TestDir)->getBufferForFile(FullPathName))
+  Inputs.Contents = Contents->get()->getBuffer().str();
+
+return Inputs;
+  }
+
+  SmallString<256> TestDir;
+  // Noticed MockFS but its member variable 'OverlayRealFileSystemForModules'
+  // implies that it will better to use RealThreadsafeFS directly.

ChuanqiXu9 wrote:

I tried but it shows it is harder. It looks like both the scanning part and the 
modules builder part require reading files from real IO. After I change 
RealThreadsafeFS to MockFS, almost all tests get failed. 

I add a FIXME here. I wish to land the series patches faster given it is too 
long.

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,355 @@
+//===--- PrerequisiteModulesTests.cpp ---*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+/// FIXME: Skip testing on windows temporarily due to the different escaping
+/// code mode.
+#ifndef _WIN32
+
+#include "ModulesBuilder.h"
+#include "ScanningProjectModules.h"
+
+#include "Annotations.h"
+#include "CodeComplete.h"
+#include "Compiler.h"
+#include "TestTU.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang::clangd {
+namespace {
+
+class MockDirectoryCompilationDatabase : public MockCompilationDatabase {
+public:
+  MockDirectoryCompilationDatabase(StringRef TestDir, const ThreadsafeFS &TFS)
+  : MockCompilationDatabase(TestDir),
+MockedCDBPtr(std::make_shared(*this)),
+TFS(TFS) {
+this->ExtraClangFlags.push_back("-std=c++20");
+this->ExtraClangFlags.push_back("-c");
+  }
+
+  void addFile(llvm::StringRef Path, llvm::StringRef Contents);
+
+  std::unique_ptr getProjectModules(PathRef) const override {
+return scanningProjectModules(MockedCDBPtr, TFS);
+  }
+
+private:
+  class MockClangCompilationDatabase : public tooling::CompilationDatabase {
+  public:
+MockClangCompilationDatabase(MockDirectoryCompilationDatabase &MCDB)
+: MCDB(MCDB) {}
+
+std::vector
+getCompileCommands(StringRef FilePath) const override {
+  std::optional Cmd =
+  MCDB.getCompileCommand(FilePath);
+  EXPECT_TRUE(Cmd);
+  return {*Cmd};
+}
+
+std::vector getAllFiles() const override { return Files; }
+
+void AddFile(StringRef File) { Files.push_back(File.str()); }
+
+  private:
+MockDirectoryCompilationDatabase &MCDB;
+std::vector Files;
+  };
+
+  std::shared_ptr MockedCDBPtr;
+  const ThreadsafeFS &TFS;
+};
+
+// Add files to the working testing directory and the compilation database.
+void MockDirectoryCompilationDatabase::addFile(llvm::StringRef Path,
+   llvm::StringRef Contents) {
+  ASSERT_FALSE(llvm::sys::path::is_absolute(Path));
+
+  SmallString<256> AbsPath(Directory);
+  llvm::sys::path::append(AbsPath, Path);
+
+  ASSERT_FALSE(
+  
llvm::sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath)));
+
+  std::error_code EC;
+  llvm::raw_fd_ostream OS(AbsPath, EC);
+  ASSERT_FALSE(EC);
+  OS << Contents;
+
+  MockedCDBPtr->AddFile(Path);
+}
+
+class PrerequisiteModulesTests : public ::testing::Test {
+protected:
+  void SetUp() override {
+ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("modules-test", 
TestDir));
+  }
+
+  void TearDown() override {
+ASSERT_FALSE(llvm::sys::fs::remove_directories(TestDir));
+  }
+
+public:
+  // Get the absolute path for file specified by Path under testing working
+  // directory.
+  std::string getFullPath(llvm::StringRef Path) {
+SmallString<128> Result(TestDir);
+llvm::sys::path::append(Result, Path);
+EXPECT_TRUE(llvm::sys::fs::exists(Result.str()));
+return Result.str().str();
+  }
+
+  std::unique_ptr getGlobalCompilationDatabase() {
+// The compilation flags with modules are much complex so it looks better
+// to use DirectoryBasedGlobalCompilationDatabase than a mocked compilation
+// database.
+DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS);
+return std::make_unique(Opts);
+  }
+
+  ParseInputs getInputs(llvm::StringRef FileName,
+const GlobalCompilationDatabase &CDB) {
+std::string FullPathName = getFullPath(FileName);
+
+ParseInputs Inputs;
+std::optional Cmd =
+CDB.getCompileCommand(FullPathName);
+EXPECT_TRUE(Cmd);
+Inputs.CompileCommand = std::move(*Cmd);
+Inputs.TFS = &TFS;
+
+if (auto Contents = TFS.view(TestDir)->getBufferForFile(FullPathName))
+  Inputs.Contents = Contents->get()->getBuffer().str();
+
+return Inputs;
+  }
+
+  SmallString<256> TestDir;
+  // Noticed MockFS but its member variable 'OverlayRealFileSystemForModules'
+  // implies that it will better to use RealThreadsafeFS directly.
+  RealThreadsafeFS TFS;
+
+  DiagnosticConsumer DiagConsumer;
+};
+
+TEST_F(PrerequisiteModulesTests, PrerequisiteModulesTest) {
+  MockDirectoryCompilationDatabase CDB(TestDir, TFS);
+
+  CDB.addFile("foo.h", R"cpp(
+inline void foo() {}
+  )cpp");
+
+  CDB.addFile("M.cppm", R"cpp(
+module;
+#include "foo.h"
+export module M;
+  )cpp");
+
+  CDB.addFile("N.cppm", R"cpp(
+export module N;
+import :Part;
+import M;
+  )cpp");
+
+  CDB.addFile("N-part.cppm", R"cpp(
+// Different name with filename intentionally.
+export module N:Part;
+  )cpp");

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,340 @@
+//===- ModulesBuilder.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+
+#include "Compiler.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+#include "clang/Serialization/ASTReader.h"

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,347 @@
+//===- ModulesBuilder.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+
+#include "Compiler.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+//   {TEMP_DIRS}/clangd/module_files/{hashed-file-name}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database.
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile) {
+  llvm::SmallString<128> HashedPrefix = llvm::sys::path::filename(MainFile);
+  // There might be multiple files with the same name in a project. So 
appending
+  // the hash value of the full path to make sure they won't conflict.
+  HashedPrefix += std::to_string(llvm::hash_value(MainFile));
+
+  llvm::SmallString<256> ResultPattern;
+
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+  llvm::sys::path::append(ResultPattern, "clangd");
+  llvm::sys::path::append(ResultPattern, "module_files");
+
+  llvm::sys::path::append(ResultPattern, HashedPrefix);
+
+  ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+  llvm::SmallString<256> Result;
+  llvm::sys::fs::createUniquePath(ResultPattern, Result,
+  /*MakeAbsolute=*/false);
+
+  llvm::sys::fs::create_directories(Result);
+  return Result;
+}
+
+// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) {
+  llvm::SmallString<128> AbsolutePath;
+  if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+  } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+  }
+  return AbsolutePath;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+  PathRef ModuleFilesPrefix) {
+  llvm::SmallString<256> ModuleFilePath(ModuleFilesPrefix);
+  auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+  llvm::sys::path::append(ModuleFilePath, PrimaryModuleName);
+  if (!PartitionName.empty()) {
+ModuleFilePath.append("-");
+ModuleFilePath.append(PartitionName);
+  }
+
+  ModuleFilePath.append(".pcm");
+  return std::string(ModuleFilePath);
+}
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+  ~FailedPrerequisiteModules() override = default;
+
+  // We shouldn't adjust the compilation commands based on
+  // FailedPrerequisiteModules.
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+  }
+
+  // FailedPrerequisiteModules can never be reused.
+  bool
+  canReuse(const CompilerInvocation &CI,
+   llvm::IntrusiveRefCntPtr) const override {
+return false;
+  }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class StandalonePrerequisiteModules : public PrerequisiteModules {
+public:
+  StandalonePrerequisiteModules() = default;
+
+  StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = 
delete;
+  StandalonePrerequisiteModules
+  operator=(const StandalonePrerequisiteModules &) = delete;
+  StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete;
+  StandalonePrerequisiteModules
+  operator=(StandalonePrerequisiteModules &&) = delete;
+
+  ~StandalonePrerequisiteModules() override = default;
+
+  void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
+// Appending all built module f

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,340 @@
+//===- ModulesBuilder.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+
+#include "Compiler.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+#include "clang/Serialization/ASTReader.h"

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -146,6 +146,8 @@ class Checker {
   ClangdLSPServer::Options Opts;
   // from buildCommand
   tooling::CompileCommand Cmd;
+  std::unique_ptr BaseCDB;
+  std::unique_ptr CDB;

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -0,0 +1,110 @@
+//===- ModulesBuilder.h --*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// TODO: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Frontend/CompilerInvocation.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include 

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

> > @Arthapz @guijiyang thanks for reporting. But could you try to give 
> > feedbacks to that repo instead of this page. I feel this page it too long 
> > now. (My browser needs seconds to load it). And this page is indeed not a 
> > good place to track that.
> > @guijiyang it looks like the reason why it can't works is that the clang 
> > tools can't handle with MSVC's option '/ifcOutput' and '/interface' . It 
> > may be better if you can compile your project with clang.exe on Windows and 
> > use a compile commands from that. And also, as far as I know, clang can't 
> > compile std module from MSSTL. Then clangd can't handle that too. Maybe we 
> > have to need to wait for clang to compile std module from STL.
> > @Arthapz For the first error, it says the module unit doesn't declare that 
> > module. Maybe it will helpful to check that. For the second error, it 
> > should be a inconsistent configuration in the clangd side. 
> > https://clang.llvm.org/docs/StandardCPlusPlusModules.html#object-definition-consistency.
> >  We skipped ODR check by default in clang but it looks like we forgot it in 
> > clangd.
> 
> Ok, i got it, and which repo can give feedback to you?

Please use this one: https://github.com/ChuanqiXu9/clangd-for-modules

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][driver] Support `--precompile` and `-fmodule-*` options in Clang-CL (PR #98761)

2024-07-15 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 commented:

Thanks. Can you add a test for this under (clang/test/Driver) and add new 
subsection for it in StandardCPlusPlusModules.rst ?

https://github.com/llvm/llvm-project/pull/98761
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Doc] Update documentation for no-transitive-change (PR #96453)

2024-07-15 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

I am going to land this in later of the week if no objections come. As 19.x is 
going to be branched.

https://github.com/llvm/llvm-project/pull/96453
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)

2024-07-15 Thread Chuanqi Xu via cfe-commits



@@ -905,6 +912,7 @@ void Preprocessor::Lex(Token &Result) {
 // This token is injected to represent the translation of '#include "a.h"'
 // into "import a.h;". Mimic the notional ';'.
 case tok::annot_module_include:
+case tok::annot_repl_input_end:

ChuanqiXu9 wrote:

I introduced  `Interpreter/cxx20-modules.cppm` but I don't have a feeling for 
change here. Just for its name, it looks slightly odd that we will still handle 
things after we see `annot_repl_input_end`. (It is an end, isn't it?)

https://github.com/llvm/llvm-project/pull/90574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

2024-07-15 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

> 
> > (FWIW, check some of the recent modules changes @ChuanqiXu9 has been 
> > working on to see that reviewer bandwidth here is pretty thin (& my 
> > experience in LLVM in general, including clang, is that reviewer bandwidth 
> > is pretty thin - though it is something we should address & I do think it 
> > might be time to change LLVM's post-commit review policy, but I think it'll 
> > be a substantial amount of work))
> 
> If you feel bandwidth for modules is pretty thin, I put myself available as a 
> reviewer, so feel free to ping me. I may not have a lot of time available for 
> fully reviewing big patches, but I can certainly help with the smaller 
> patches such as this one.

Thanks. I'll try to add you for future reviewings.

> > > That's a pretty substantial policy change to propose, and this probably 
> > > isn't the place to propose/discuss it. If that's your intent, probably 
> > > best to take that up on discord.
> 
> I am not proposing a policy change. I believe the current policy is aimed at 
> giving an escape hatch for projects which there is basically one or two 
> active developers. I am pointing out that I believe we don't need for that 
> escape hatch to apply to any parts of clang currently.

To be honest, this is not what I feel at least for coroutines and modules for a 
long time. I had multiple experience that some patches doesn't get reviewed for 
months. I still believe it will work better if I can push some simple patches 
directly.

https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

2024-07-15 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

@Arthapz @guijiyang thanks for reporting. But could you try to give feedbacks 
to that repo instead of this page. I feel this page it too long now. (My 
browser needs seconds to load it). And this page is indeed not a good place to 
track that.

---

@guijiyang  it looks like the reason why it can't works is that the clang tools 
can't handle with MSVC's option '/ifcOutput' and '/interface' . It may be 
better if you can compile your project with clang.exe on Windows and use a 
compile commands from that. And also, as far as I know, clang can't compile std 
module from MSSTL. Then clangd can't handle that too. Maybe we  have to need to 
wait for clang to compile std module from STL.

---

@Arthapz For the first error, it says the module unit doesn't declare that 
module. Maybe it will helpful to check that. For the second error, it should be 
a inconsistent configuration in the clangd side. 
https://clang.llvm.org/docs/StandardCPlusPlusModules.html#object-definition-consistency.
 We skipped ODR check by default in clang but it looks like we forgot it in 
clangd.

https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e0d66c2 - [C++20] [Modules] Allow export redeclarations within language linkage

2024-07-11 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2024-07-12T13:55:56+08:00
New Revision: e0d66c242462d63d99a5324f9799ae5f84f2d84f

URL: 
https://github.com/llvm/llvm-project/commit/e0d66c242462d63d99a5324f9799ae5f84f2d84f
DIFF: 
https://github.com/llvm/llvm-project/commit/e0d66c242462d63d99a5324f9799ae5f84f2d84f.diff

LOG: [C++20] [Modules] Allow export redeclarations within language linkage

Close https://github.com/llvm/llvm-project/issues/98583

Currently, clang will reject the following code:

```
export module mod;
extern "C++" void func();
export extern "C++" {
void func();
}
```

while both MSVC and GCC accept it. Although clang's behavior matches the
current wording, from the discussion, the consensus is that we should
accept the above example from the intention. Since the intention to not
allow export redeclaration which is not exported is to make the linkage
clear. But it doesn't matter with the declarations within global module.

Added: 
clang/test/Modules/export-redecl-in-language-linkage.cppm

Modified: 
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index e3377bef2adeb..80b5a8cd4bae6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1676,6 +1676,15 @@ bool Sema::CheckRedeclarationExported(NamedDecl *New, 
NamedDecl *Old) {
   if (IsOldExported)
 return false;
 
+  // If the Old declaration are not attached to named modules
+  // and the New declaration are attached to global module.
+  // It should be fine to allow the export since it doesn't change
+  // the linkage of declarations. See
+  // https://github.com/llvm/llvm-project/issues/98583 for details.
+  if (!Old->isInNamedModule() && New->getOwningModule() &&
+  New->getOwningModule()->isImplicitGlobalModule())
+return false;
+
   assert(IsNewExported);
 
   auto Lk = Old->getFormalLinkage();

diff  --git a/clang/test/Modules/export-redecl-in-language-linkage.cppm 
b/clang/test/Modules/export-redecl-in-language-linkage.cppm
new file mode 100644
index 0..c6a4932f5b071
--- /dev/null
+++ b/clang/test/Modules/export-redecl-in-language-linkage.cppm
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify -fsyntax-only
+
+// expected-no-diagnostics
+export module mod;
+extern "C++" void func();
+export extern "C++" {
+void func();
+}



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

[clang] d384267 - [NFC] [Modules] Introduce 'DeclBase::isInNamedModule' interface

2024-07-11 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2024-07-12T13:35:56+08:00
New Revision: d384267ad0d5494832f7f53b888c3968b7e688a8

URL: 
https://github.com/llvm/llvm-project/commit/d384267ad0d5494832f7f53b888c3968b7e688a8
DIFF: 
https://github.com/llvm/llvm-project/commit/d384267ad0d5494832f7f53b888c3968b7e688a8.diff

LOG: [NFC] [Modules] Introduce 'DeclBase::isInNamedModule' interface

This patch introduces DeclBase::isInNamedModule API to ease the use
of modules slightly.

Added: 


Modified: 
clang/include/clang/AST/DeclBase.h
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 06ffc2ce09b89..6c711cfe7927b 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -673,6 +673,9 @@ class alignas(8) Decl {
   /// Whether this declaration comes from explicit global module.
   bool isFromExplicitGlobalModule() const;
 
+  /// Whether this declaration comes from a named module.
+  bool isInNamedModule() const;
+
   /// Return true if this declaration has an attribute which acts as
   /// definition of the entity, such as 'alias' or 'ifunc'.
   bool hasDefiningAttr() const;

diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index ecccab08cbaab..490c4a2fc525c 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1181,13 +1181,6 @@ Linkage NamedDecl::getLinkageInternal() const {
   .getLinkage();
 }
 
-/// Determine whether D is attached to a named module.
-static bool isInNamedModule(const NamedDecl *D) {
-  if (auto *M = D->getOwningModule())
-return M->isNamedModule();
-  return false;
-}
-
 static bool isExportedFromModuleInterfaceUnit(const NamedDecl *D) {
   // FIXME: Handle isModulePrivate.
   switch (D->getModuleOwnershipKind()) {
@@ -1197,7 +1190,7 @@ static bool isExportedFromModuleInterfaceUnit(const 
NamedDecl *D) {
 return false;
   case Decl::ModuleOwnershipKind::Visible:
   case Decl::ModuleOwnershipKind::VisibleWhenImported:
-return isInNamedModule(D);
+return D->isInNamedModule();
   }
   llvm_unreachable("unexpected module ownership kind");
 }
@@ -1215,7 +1208,7 @@ Linkage NamedDecl::getFormalLinkage() const {
   // [basic.namespace.general]/p2
   //   A namespace is never attached to a named module and never has a name 
with
   //   module linkage.
-  if (isInNamedModule(this) && InternalLinkage == Linkage::External &&
+  if (isInNamedModule() && InternalLinkage == Linkage::External &&
   !isExportedFromModuleInterfaceUnit(
   cast(this->getCanonicalDecl())) &&
   !isa(this))

diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index eef946e3aea2e..ef2c57e6204dc 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1145,6 +1145,10 @@ bool Decl::isFromExplicitGlobalModule() const {
   return getOwningModule() && getOwningModule()->isExplicitGlobalModule();
 }
 
+bool Decl::isInNamedModule() const {
+  return getOwningModule() && getOwningModule()->isNamedModule();
+}
+
 static Decl::Kind getKind(const Decl *D) { return D->getKind(); }
 static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); }
 

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 66eeaa8e6f777..e3377bef2adeb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10094,7 +10094,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, 
DeclContext *DC,
 // check at the end of the TU (or when the PMF starts) to see that we
 // have a definition at that point.
 if (isInline && !D.isFunctionDefinition() && getLangOpts().CPlusPlus20 &&
-NewFD->hasOwningModule() && NewFD->getOwningModule()->isNamedModule()) 
{
+NewFD->isInNamedModule()) {
   PendingInlineFuncDecls.insert(NewFD);
 }
   }



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

[clang] [Clang] Don't crash if input file is not a module. (PR #98439)

2024-07-11 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/98439
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Don't crash if input file is not a module. (PR #98439)

2024-07-11 Thread Chuanqi Xu via cfe-commits



@@ -226,6 +226,7 @@ def err_module_map_not_found : Error<"module map file '%0' 
not found">,
 def err_missing_module_name : Error<
   "no module name provided; specify one with -fmodule-name=">,
   DefaultFatal;
+def err_file_is_not_module : Error<"file '%0' is not a module">, DefaultFatal;

ChuanqiXu9 wrote:

OK, I'll commit it.

https://github.com/llvm/llvm-project/pull/98439
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Don't crash if input file is not a module. (PR #98439)

2024-07-11 Thread Chuanqi Xu via cfe-commits



@@ -226,6 +226,7 @@ def err_module_map_not_found : Error<"module map file '%0' 
not found">,
 def err_missing_module_name : Error<
   "no module name provided; specify one with -fmodule-name=">,
   DefaultFatal;
+def err_file_is_not_module : Error<"file '%0' is not a module">, DefaultFatal;

ChuanqiXu9 wrote:

```suggestion
def err_file_is_not_module : Error<"file '%0' is not a module file">, 
DefaultFatal;
```

https://github.com/llvm/llvm-project/pull/98439
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Don't crash if input file is not a module. (PR #98439)

2024-07-11 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/98439
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Don't crash if input file is not a module. (PR #98439)

2024-07-11 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/98439
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

2024-07-10 Thread Chuanqi Xu via cfe-commits


ChuanqiXu9 wrote:

> The reproducer turned out to be pretty simple:
> 
> ```c++
> export module a;
> module :private;
> static void f() {}
> void g() {
>   f();
> }
> ```
> 
> Compiles without that patch, otherwise produces:
> 
> ```
> error: no matching function for call to 'f'
> ```
> 
> > Oh, sorry, I took another look at the commit and it looks the change makes 
> > it not a NFC change is this line: 
> > [99873b3#diff-6fe53759e8d797c328c73ada5f3324c6248a8634ef36131c7eb2b9d89192bb64R6514](https://github.com/llvm/llvm-project/commit/99873b35da7ecb905143c8a6b8deca4d4416f1a9#diff-6fe53759e8d797c328c73ada5f3324c6248a8634ef36131c7eb2b9d89192bb64R6514)
> > This shouldn't be in that commit but in this commit. It is not intentional. 
> > I guess we can't observe that if we put that in this PR too. And that 
> > change looks not bad. So maybe it makes something already bad to show up.
> 
> Even without that change, the other changes are too complex, and there are 
> other suspicious things you wouldn't expect in an NFC commit which doesn't 
> call this out specifically, like the removal of that whole block with the 
> FIXME included.
> 
> I think the appropriate tag for such commits would be NFCI, and should still 
> require PR and review.

Thanks for the reproducer and the information about NFCI. It looks like I took 
an oversight in the refactoring of `isInAnotherModuleUnit` that I missed the 
case about private module fragment.

https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] static data members of template classes should be allowed in header units (PR #98309)

2024-07-10 Thread Chuanqi Xu via cfe-commits


https://github.com/ChuanqiXu9 approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/98309
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

< 1 2 3 4 5 6 7 8 9 10 >

201 - 300 of 1664 matches

Mail list logo