[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)

[Chuanqi Xu via cfe-commits]

https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/89751
___
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)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

@rjmccall @dwblaikie ping

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] [NFC] Move DeclID from serialization/ASTBitCodes.h to AST/DeclID.h (PR #89873)

[Chuanqi Xu via cfe-commits]

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

Previously, the DeclID is defined in serialization/ASTBitCodes.h under 
clang::serialization namespace. However, actually the DeclID is not purely used 
in serialization part. The DeclID is already widely used in AST and all around 
the clang project via classes like `LazyPtrDecl` or calling 
`ExternalASTSource::getExernalDecl()`. All such uses are via the raw underlying 
type of `DeclID` as `uint32_t`. This is not pretty good.

This patch moves the DeclID class family to a new header `AST/DeclID.h` so that 
the whole project can use the wrapped class `DeclID`, `GlobalDeclID` and 
`LocalDeclID` instead of the raw underlying type. This can improve the 
readability and the type safety.

>From 3c8e76dcf7746d7ede5434e0fbf025802590bd68 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 24 Apr 2024 13:35:01 +0800
Subject: [PATCH] [NFC] Move DeclID from serialization/ASTBitCodes.h to
 AST/DeclID.h

Previously, the DeclID is defined in serialization/ASTBitCodes.h under
clang::serialization namespace. However, actually the DeclID is not
purely used in serialization part. The DeclID is already widely used in
AST and all around the clang project via classes like `LazyPtrDecl` or
calling `ExternalASTSource::getExernalDecl()`. All such uses are via the
raw underlying type of `DeclID` as `uint32_t`. This is not pretty good.

This patch moves the DeclID class family to a new header `AST/DeclID.h`
so that the whole project can use the wrapped class `DeclID`,
`GlobalDeclID` and `LocalDeclID` instead of the raw underlying type.
This can improve the readability and the type safety.
---
 clang/include/clang/AST/ASTContext.h  |   4 +-
 clang/include/clang/AST/DeclBase.h|   4 +-
 clang/include/clang/AST/DeclID.h  | 175 ++
 clang/include/clang/AST/DeclTemplate.h|   2 +-
 clang/include/clang/AST/ExternalASTSource.h   |   4 +-
 clang/include/clang/Frontend/ASTUnit.h|   2 +-
 .../clang/Frontend/MultiplexConsumer.h|   2 +-
 .../clang/Sema/MultiplexExternalSemaSource.h  |   2 +-
 .../include/clang/Serialization/ASTBitCodes.h | 161 +---
 .../ASTDeserializationListener.h  |   2 +-
 clang/include/clang/Serialization/ASTReader.h | 126 ++---
 .../clang/Serialization/ASTRecordReader.h |   6 +-
 clang/include/clang/Serialization/ASTWriter.h |  22 +--
 .../include/clang/Serialization/ModuleFile.h  |   8 +-
 clang/lib/AST/ASTContext.cpp  |   3 +-
 clang/lib/AST/Decl.cpp|  46 ++---
 clang/lib/AST/DeclBase.cpp|   4 +-
 clang/lib/AST/DeclCXX.cpp |  63 +++
 clang/lib/AST/DeclFriend.cpp  |   2 +-
 clang/lib/AST/DeclObjC.cpp|  24 +--
 clang/lib/AST/DeclOpenMP.cpp  |  18 +-
 clang/lib/AST/DeclTemplate.cpp|  41 ++--
 clang/lib/AST/ExternalASTSource.cpp   |   2 +-
 clang/lib/Frontend/ASTUnit.cpp|   4 +-
 clang/lib/Frontend/FrontendAction.cpp |   6 +-
 clang/lib/Frontend/MultiplexConsumer.cpp  |   3 +-
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |   2 +-
 clang/lib/Serialization/ASTReader.cpp |  16 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  18 +-
 clang/lib/Serialization/ASTWriter.cpp |   4 +-
 clang/lib/Serialization/ASTWriterDecl.cpp |   4 +-
 31 files changed, 382 insertions(+), 398 deletions(-)
 create mode 100644 clang/include/clang/AST/DeclID.h

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index d5ed20ff50157d..ecec9bfcf30079 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -455,7 +455,7 @@ class ASTContext : public RefCountedBase {
   /// initialization of another module).
   struct PerModuleInitializers {
 llvm::SmallVector Initializers;
-llvm::SmallVector LazyInitializers;
+llvm::SmallVector LazyInitializers;
 
 void resolve(ASTContext &Ctx);
   };
@@ -1059,7 +1059,7 @@ class ASTContext : public RefCountedBase {
   /// or an ImportDecl nominating another module that has initializers.
   void addModuleInitializer(Module *M, Decl *Init);
 
-  void addLazyModuleInitializers(Module *M, ArrayRef IDs);
+  void addLazyModuleInitializers(Module *M, ArrayRef IDs);
 
   /// Get the initializations to perform when importing a module, if any.
   ArrayRef getModuleInitializers(Module *M);
diff --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index d8cafc3d81526e..474e51c1df6d68 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -15,6 +15,7 @@
 
 #include "clang/AST/ASTDumperUtils.h"
 #include "clang/AST/AttrIterator.h"
+#include "clang/AST/DeclID.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/SelectorLocationsKind.h"
 #include "clang/Basic/Identifie

[clang] [NFC] Move DeclID from serialization/ASTBitCodes.h to AST/DeclID.h (PR #89873)

[Chuanqi Xu via cfe-commits]

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

[clang] [NFC] Move DeclID from serialization/ASTBitCodes.h to AST/DeclID.h (PR #89873)

[Chuanqi Xu via cfe-commits]

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

>From d83b9cda6c7d943e90c324fa2c1c7e7ffaf88e1c Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 24 Apr 2024 13:35:01 +0800
Subject: [PATCH] [NFC] Move DeclID from serialization/ASTBitCodes.h to
 AST/DeclID.h

Previously, the DeclID is defined in serialization/ASTBitCodes.h under
clang::serialization namespace. However, actually the DeclID is not
purely used in serialization part. The DeclID is already widely used in
AST and all around the clang project via classes like `LazyPtrDecl` or
calling `ExternalASTSource::getExernalDecl()`. All such uses are via the
raw underlying type of `DeclID` as `uint32_t`. This is not pretty good.

This patch moves the DeclID class family to a new header `AST/DeclID.h`
so that the whole project can use the wrapped class `DeclID`,
`GlobalDeclID` and `LocalDeclID` instead of the raw underlying type.
This can improve the readability and the type safety.
---
 clang/include/clang/AST/ASTContext.h  |   4 +-
 clang/include/clang/AST/DeclBase.h|   4 +-
 clang/include/clang/AST/DeclID.h  | 177 ++
 clang/include/clang/AST/DeclTemplate.h|   2 +-
 clang/include/clang/AST/ExternalASTSource.h   |   4 +-
 clang/include/clang/Frontend/ASTUnit.h|   2 +-
 .../clang/Frontend/MultiplexConsumer.h|   2 +-
 .../clang/Sema/MultiplexExternalSemaSource.h  |   2 +-
 .../include/clang/Serialization/ASTBitCodes.h | 161 +---
 .../ASTDeserializationListener.h  |   2 +-
 clang/include/clang/Serialization/ASTReader.h | 126 ++---
 .../clang/Serialization/ASTRecordReader.h |   6 +-
 clang/include/clang/Serialization/ASTWriter.h |  22 +--
 .../include/clang/Serialization/ModuleFile.h  |   8 +-
 clang/lib/AST/ASTContext.cpp  |   3 +-
 clang/lib/AST/Decl.cpp|  46 ++---
 clang/lib/AST/DeclBase.cpp|   4 +-
 clang/lib/AST/DeclCXX.cpp |  63 +++
 clang/lib/AST/DeclFriend.cpp  |   2 +-
 clang/lib/AST/DeclObjC.cpp|  24 +--
 clang/lib/AST/DeclOpenMP.cpp  |  18 +-
 clang/lib/AST/DeclTemplate.cpp|  41 ++--
 clang/lib/AST/ExternalASTSource.cpp   |   2 +-
 clang/lib/Frontend/ASTUnit.cpp|   4 +-
 clang/lib/Frontend/FrontendAction.cpp |   6 +-
 clang/lib/Frontend/MultiplexConsumer.cpp  |   3 +-
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |   2 +-
 clang/lib/Serialization/ASTReader.cpp |  16 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  18 +-
 clang/lib/Serialization/ASTWriter.cpp |   4 +-
 clang/lib/Serialization/ASTWriterDecl.cpp |   4 +-
 31 files changed, 384 insertions(+), 398 deletions(-)
 create mode 100644 clang/include/clang/AST/DeclID.h

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index d5ed20ff50157d..ecec9bfcf30079 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -455,7 +455,7 @@ class ASTContext : public RefCountedBase {
   /// initialization of another module).
   struct PerModuleInitializers {
 llvm::SmallVector Initializers;
-llvm::SmallVector LazyInitializers;
+llvm::SmallVector LazyInitializers;
 
 void resolve(ASTContext &Ctx);
   };
@@ -1059,7 +1059,7 @@ class ASTContext : public RefCountedBase {
   /// or an ImportDecl nominating another module that has initializers.
   void addModuleInitializer(Module *M, Decl *Init);
 
-  void addLazyModuleInitializers(Module *M, ArrayRef IDs);
+  void addLazyModuleInitializers(Module *M, ArrayRef IDs);
 
   /// Get the initializations to perform when importing a module, if any.
   ArrayRef getModuleInitializers(Module *M);
diff --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index d8cafc3d81526e..474e51c1df6d68 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -15,6 +15,7 @@
 
 #include "clang/AST/ASTDumperUtils.h"
 #include "clang/AST/AttrIterator.h"
+#include "clang/AST/DeclID.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/SelectorLocationsKind.h"
 #include "clang/Basic/IdentifierTable.h"
@@ -239,9 +240,6 @@ class alignas(8) Decl {
 ModulePrivate
   };
 
-  /// An ID number that refers to a declaration in an AST file.
-  using DeclID = uint32_t;
-
 protected:
   /// The next declaration within the same lexical
   /// DeclContext. These pointers form the linked list that is
diff --git a/clang/include/clang/AST/DeclID.h b/clang/include/clang/AST/DeclID.h
new file mode 100644
index 00..e2c6dd65e86bc3
--- /dev/null
+++ b/clang/include/clang/AST/DeclID.h
@@ -0,0 +1,177 @@
+//===--- DeclID.h - ID number for deserialized declarations  *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See ht

[clang] [NFC] Move DeclID from serialization/ASTBitCodes.h to AST/DeclID.h (PR #89873)

[Chuanqi Xu via cfe-commits]

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

[clang] fe47e8f - [NFC] [ASTUnit] [Serialization] Transalte local decl ID to global decl ID before consuming

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-04-25T15:55:46+08:00
New Revision: fe47e8ff3ae7fc8975eaade6bfa6679737c28b93

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

LOG: [NFC] [ASTUnit] [Serialization] Transalte local decl ID to global decl ID 
before consuming

Discovered from
https://github.com/llvm/llvm-project/commit/d86cc73bbfd9a22d9a0d498d72c9b2ee235128e9.

There is a potential issue of using DeclID in ASTUnit. ASTUnit may
record the declaration ID from ASTWriter. And after loading the
preamble, the ASTUnit may consume the recorded declaration ID directly
in ExternalASTSource. This is not good. According to the design, all
local declaration ID consumed in ASTReader need to be translated by
`ASTReader::getGlobaldeclID()`.

This will be problematic if we changed the encodings of declaration IDs or if we
make preamble to work more complexly.

Added: 


Modified: 
clang/lib/Frontend/ASTUnit.cpp

Removed: 




diff  --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 2f75313e8a4c50..1b93588553a276 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -1467,13 +1467,12 @@ void ASTUnit::RealizeTopLevelDeclsFromPreamble() {
 
   std::vector Resolved;
   Resolved.reserve(TopLevelDeclsInPreamble.size());
-  ExternalASTSource &Source = *getASTContext().getExternalSource();
+  // The module file of the preamble.
+  serialization::ModuleFile &MF = 
Reader->getModuleManager().getPrimaryModule();
   for (const auto TopLevelDecl : TopLevelDeclsInPreamble) {
 // Resolve the declaration ID to an actual declaration, possibly
 // deserializing the declaration in the process.
-//
-// FIMXE: We shouldn't convert a LocalDeclID to GlobalDeclID directly.
-if (Decl *D = Source.GetExternalDecl(GlobalDeclID(TopLevelDecl.get(
+if (Decl *D = Reader->GetDecl(Reader->getGlobalDeclID(MF, TopLevelDecl)))
   Resolved.push_back(D);
   }
   TopLevelDeclsInPreamble.clear();



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

[clang] [Modules] No transitive source location change (PR #86912)

[Chuanqi Xu via cfe-commits]

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

>From 2c20a6200fb2790b3a891ffc8c43682c113c7e8a Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Mon, 18 Mar 2024 08:36:55 +0800
Subject: [PATCH] [Modules] No transitive source location change

---
 clang/include/clang/Basic/SourceLocation.h|  1 +
 .../include/clang/Serialization/ASTBitCodes.h | 56 +---
 clang/include/clang/Serialization/ASTReader.h | 48 ++
 clang/include/clang/Serialization/ASTWriter.h |  4 +
 .../include/clang/Serialization/ModuleFile.h  | 14 ++-
 .../Serialization/SourceLocationEncoding.h| 91 +--
 clang/lib/Frontend/ASTUnit.cpp|  2 -
 clang/lib/Serialization/ASTReader.cpp | 57 
 clang/lib/Serialization/ASTReaderDecl.cpp |  2 +-
 clang/lib/Serialization/ASTWriter.cpp | 41 +++--
 clang/lib/Serialization/ASTWriterDecl.cpp |  8 +-
 clang/lib/Serialization/ModuleFile.cpp|  1 -
 .../no-transitive-source-location-change.cppm | 69 ++
 clang/test/Modules/pr61067.cppm   | 25 -
 .../SourceLocationEncodingTest.cpp| 12 ++-
 15 files changed, 269 insertions(+), 162 deletions(-)
 create mode 100644 clang/test/Modules/no-transitive-source-location-change.cppm

diff --git a/clang/include/clang/Basic/SourceLocation.h 
b/clang/include/clang/Basic/SourceLocation.h
index 00b1e0fa855b7a..7a0f5ba8d1270b 100644
--- a/clang/include/clang/Basic/SourceLocation.h
+++ b/clang/include/clang/Basic/SourceLocation.h
@@ -90,6 +90,7 @@ class SourceLocation {
   friend class ASTWriter;
   friend class SourceManager;
   friend struct llvm::FoldingSetTrait;
+  friend class SourceLocationEncoding;
 
 public:
   using UIntTy = uint32_t;
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 186c3b722ced16..94a3d24d47926b 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -23,6 +23,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Serialization/SourceLocationEncoding.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Bitstream/BitCodes.h"
 #include 
@@ -167,45 +168,38 @@ const unsigned int NUM_PREDEF_SUBMODULE_IDS = 1;
 
 /// Source range/offset of a preprocessed entity.
 struct PPEntityOffset {
+  using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
   /// Raw source location of beginning of range.
-  SourceLocation::UIntTy Begin;
+  RawLocEncoding Begin;
 
   /// Raw source location of end of range.
-  SourceLocation::UIntTy End;
+  RawLocEncoding End;
 
   /// Offset in the AST file relative to ModuleFile::MacroOffsetsBase.
   uint32_t BitOffset;
 
-  PPEntityOffset(SourceRange R, uint32_t BitOffset)
-  : Begin(R.getBegin().getRawEncoding()), End(R.getEnd().getRawEncoding()),
-BitOffset(BitOffset) {}
-
-  SourceLocation getBegin() const {
-return SourceLocation::getFromRawEncoding(Begin);
-  }
+  PPEntityOffset(RawLocEncoding Begin, RawLocEncoding End, uint32_t BitOffset)
+  : Begin(Begin), End(End), BitOffset(BitOffset) {}
 
-  SourceLocation getEnd() const {
-return SourceLocation::getFromRawEncoding(End);
-  }
+  RawLocEncoding getBegin() const { return Begin; }
+  RawLocEncoding getEnd() const { return End; }
 };
 
 /// Source range of a skipped preprocessor region
 struct PPSkippedRange {
+  using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
   /// Raw source location of beginning of range.
-  SourceLocation::UIntTy Begin;
+  RawLocEncoding Begin;
   /// Raw source location of end of range.
-  SourceLocation::UIntTy End;
+  RawLocEncoding End;
 
-  PPSkippedRange(SourceRange R)
-  : Begin(R.getBegin().getRawEncoding()), End(R.getEnd().getRawEncoding()) 
{
-  }
+  PPSkippedRange(RawLocEncoding Begin, RawLocEncoding End)
+  : Begin(Begin), End(End) {}
 
-  SourceLocation getBegin() const {
-return SourceLocation::getFromRawEncoding(Begin);
-  }
-  SourceLocation getEnd() const {
-return SourceLocation::getFromRawEncoding(End);
-  }
+  RawLocEncoding getBegin() const { return Begin; }
+  RawLocEncoding getEnd() const { return End; }
 };
 
 /// Offset in the AST file. Use splitted 64-bit integer into low/high
@@ -231,8 +225,10 @@ struct UnderalignedInt64 {
 
 /// Source location and bit offset of a declaration.
 struct DeclOffset {
+  using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
   /// Raw source location.
-  SourceLocation::UIntTy Loc = 0;
+  RawLocEncoding RawLoc = 0;
 
   /// Offset relative to the start of the DECLTYPES_BLOCK block. Keep
   /// structure alignment 32-bit and avoid padding gap because undefined
@@ -240,17 +236,15 @@ struct DeclOffset {
   UnderalignedInt64 BitOffset;
 
   DeclOffset() = default;
-  DeclOffset(SourceLocation Loc, uint64_t BitOffset,
- uint

[clang] [Modules] No transitive source location change (PR #86912)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> The changes LGTM, don't want to block this on my remaining nits.

Thanks for reviewing this.

> 
> I believe @Bigcheese wanted to test test impact on PCM size on our side 
> before this lands.

I've rebased this with main. I'll wait for the results from @Bigcheese 

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

[clang] [Serialization] Load Specializations Lazily (PR #76774)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

Given we're pursuing https://github.com/llvm/llvm-project/pull/83237 series. 
I'll close this one.

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

[clang] [Serialization] Load Specializations Lazily (PR #76774)

[Chuanqi Xu via cfe-commits]

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

[clang] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one. (PR #83108)

[Chuanqi Xu via cfe-commits]

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

>From be1c83fb885536c3e65657c6549bd20dd29d9649 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev 
Date: Sun, 7 Jan 2018 15:16:11 +0200
Subject: [PATCH 1/3] D41416: [modules] [pch] Do not deserialize all lazy
 template specializations when looking for one.

---
 clang/include/clang/AST/DeclTemplate.h|  36 +++-
 clang/lib/AST/DeclTemplate.cpp| 100 +-
 clang/lib/AST/ODRHash.cpp |  15 
 clang/lib/Serialization/ASTReader.cpp |  25 --
 clang/lib/Serialization/ASTReaderDecl.cpp |  46 ++
 clang/lib/Serialization/ASTWriter.cpp |  21 -
 clang/lib/Serialization/ASTWriterDecl.cpp |  76 +---
 7 files changed, 249 insertions(+), 70 deletions(-)

diff --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 3ee03eebdb8ca4..24578cfdaa7c7c 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -256,6 +256,9 @@ class TemplateArgumentList final
   TemplateArgumentList(const TemplateArgumentList &) = delete;
   TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;
 
+  /// Create hash for the given arguments.
+  static unsigned ComputeODRHash(ArrayRef Args);
+
   /// Create a new template argument list that copies the given set of
   /// template arguments.
   static TemplateArgumentList *CreateCopy(ASTContext &Context,
@@ -730,6 +733,26 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   }
 
   void anchor() override;
+  struct LazySpecializationInfo {
+GlobalDeclID DeclID = GlobalDeclID();
+unsigned ODRHash = ~0U;
+bool IsPartial = false;
+LazySpecializationInfo(GlobalDeclID ID, unsigned Hash = ~0U,
+   bool Partial = false)
+  : DeclID(ID), ODRHash(Hash), IsPartial(Partial) { }
+LazySpecializationInfo() { }
+bool operator<(const LazySpecializationInfo &Other) const {
+  return DeclID < Other.DeclID;
+}
+bool operator==(const LazySpecializationInfo &Other) const {
+  assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
+ "Hashes differ!");
+  assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
+ "Both must be the same kinds!");
+  return DeclID == Other.DeclID;
+}
+  };
+
 protected:
   template  struct SpecEntryTraits {
 using DeclType = EntryType;
@@ -770,7 +793,12 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 return SpecIterator(isEnd ? Specs.end() : Specs.begin());
   }
 
-  void loadLazySpecializationsImpl() const;
+  void loadLazySpecializationsImpl(bool OnlyPartial = false) const;
+
+  void loadLazySpecializationsImpl(llvm::ArrayRef Args,
+   TemplateParameterList *TPL = nullptr) const;
+
+  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
 
   template 
   typename SpecEntryTraits::DeclType*
@@ -797,7 +825,7 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 ///
 /// The first value in the array is the number of specializations/partial
 /// specializations that follow.
-GlobalDeclID *LazySpecializations = nullptr;
+LazySpecializationInfo *LazySpecializations = nullptr;
 
 /// The set of "injected" template arguments used within this
 /// template.
@@ -2284,7 +2312,7 @@ class ClassTemplateDecl : public RedeclarableTemplateDecl 
{
   friend class TemplateDeclInstantiator;
 
   /// Load any lazily-loaded specializations from the external source.
-  void LoadLazySpecializations() const;
+  void LoadLazySpecializations(bool OnlyPartial = false) const;
 
   /// Get the underlying class declarations of the template.
   CXXRecordDecl *getTemplatedDecl() const {
@@ -3056,7 +3084,7 @@ class VarTemplateDecl : public RedeclarableTemplateDecl {
   friend class ASTDeclWriter;
 
   /// Load any lazily-loaded specializations from the external source.
-  void LoadLazySpecializations() const;
+  void LoadLazySpecializations(bool OnlyPartial = false) const;
 
   /// Get the underlying variable declarations of the template.
   VarDecl *getTemplatedDecl() const {
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index d27a30e0c5fce1..1afd7b4550c917 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -20,6 +20,8 @@
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
+#include "clang/AST/ODRHash.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/LLVM.h"
@@ -331,17 +333,46 @@ RedeclarableTemplateDecl::CommonBase 
*RedeclarableTemplateDecl::getCommonPtr() c
   return Common;
 }
 
-void RedeclarableTemplateDecl::loadLazySpecializationsImpl() const {
+void RedeclarableTemplateDecl::loadLazySpecializationsImpl(
+ 

[clang] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one. (PR #83108)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

Rebased with main.

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

[clang] [Modules] Detect ODR mismatches for enums in non-C++ like in C++. (PR #90298)

[Chuanqi Xu via cfe-commits]

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

I have no idea why it was. But the current change looks pretty fine to me.

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

[clang] [llvm] [Pipelines] Do not run CoroSplit and CoroCleanup in LTO pre-link pipeline (PR #90310)

[Chuanqi Xu via cfe-commits]

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

Feel not bad.

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -530,43 +527,43 @@ Now the linkage name of ``NS::foo()`` will be 
``_ZN2NS3fooEv``.
 Module Initializers
 ~~~
 
-All the importable module units are required to emit an initializer function.
-The initializer function should contain calls to importing modules first and
-all the dynamic-initializers in the current module unit then.
-
-Translation units explicitly or implicitly importing named modules must call
-the initializer functions of the imported named modules within the sequence of
-the dynamic-initializers in the TU. Initializations of entities at namespace
-scope are appearance-ordered. This (recursively) extends into imported modules
-at the point of appearance of the import declaration.
+All importable module units are required to emit an initializer function. The

ChuanqiXu9 wrote:

They are required to handle the dynamic initializations of non-inline variables 
in the module unit. But the importable module units have to emit the 
initializer even if there is no dynamic initialization. Otherwise, the importer 
may calling a non-exist function.

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -312,75 +300,76 @@ So all of the following name is not valid by default:
 __test
 // and so on ...
 
-If you still want to use the reserved module names for any reason, use
-``-Wno-reserved-module-identifier`` to suppress the warning.
+Using a reserved module name is strongly discouraged, but
+``-Wno-reserved-module-identifier`` can be used to suppress the warning.
 
-How to specify the dependent BMIs
-~
+Specifying dependent BMIs
+~
 
-There are 3 methods to specify the dependent BMIs:
+There are 3 ways to specify a dependent BMI:

ChuanqiXu9 wrote:

If we have the following code:

```
// foo.cc
import a;
...
```

then we need a BMI of module `a` to compile `foo.cc`. Here the BMI of module 
`a` is the dependent BMI for `foo.cc`. I feel this clear. But if we don't think 
so, we can add a definition for that.

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -633,36 +630,37 @@ example:
   // module M's interface, so is discarded
   int c = use_h();   // OK
 
-In the above example, the function definition of ``N::g`` is elided from the 
Reduced
-BMI of ``M.cppm``. Then the use of ``use_g`` in ``M-impl.cpp`` fails
-to instantiate. For such issues, users can add references to ``N::g`` in the 
module purview
-of ``M.cppm`` to make sure it is reachable, e.g., ``using N::g;``.
-
-We think the Reduced BMI is the correct direction. But given it is a drastic 
change,
-we'd like to make it experimental first to avoid breaking existing users. The 
roadmap
-of Reduced BMI may be:
-
-1. ``-fexperimental-modules-reduced-bmi`` is opt in for 1~2 releases. The 
period depends
-on testing feedbacks.
-2. We would announce Reduced BMI is not experimental and introduce 
``-fmodules-reduced-bmi``.
-and suggest users to enable this mode. This may takes 1~2 releases too.
-3. Finally we will enable this by default. When that time comes, the term BMI 
will refer to
-the reduced BMI today and the Full BMI will only be meaningful to build 
systems which
-loves to support two phase compilations.
+In the above example, the function definition of ``N::g`` is elided from the
+Reduced BMI of ``M.cppm``. Then the use of ``use_g`` in ``M-impl.cpp``
+fails to instantiate. For such issues, users can add references to ``N::g`` in
+the module purview of ``M.cppm`` to ensure it is reachable, e.g.
+``using N::g;``.
+
+Long-term, Clang is likely to make Reduced BMIs the default rather than Full
+BMIs. Because it would be a drastic change of user interface, it is initially

ChuanqiXu9 wrote:

```suggestion
BMIs. Because it would be a drastic change, it is initially
```

not only the `user interface`, it requires some fundamental changes in the 
serializer, so there might be some bugs in the implementation.

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -530,43 +527,43 @@ Now the linkage name of ``NS::foo()`` will be 
``_ZN2NS3fooEv``.
 Module Initializers
 ~~~
 
-All the importable module units are required to emit an initializer function.
-The initializer function should contain calls to importing modules first and
-all the dynamic-initializers in the current module unit then.
-
-Translation units explicitly or implicitly importing named modules must call
-the initializer functions of the imported named modules within the sequence of
-the dynamic-initializers in the TU. Initializations of entities at namespace
-scope are appearance-ordered. This (recursively) extends into imported modules
-at the point of appearance of the import declaration.
+All importable module units are required to emit an initializer function. The
+initializer function emits calls to imported modules first followed by calls
+to all to dynamic initializers in the current module unit.
 
-It is allowed to omit calls to importing modules if it is known empty.
+Translation units that explicitly or implicitly import a named module must call
+the initializer functions of the imported named module within the sequence of
+the dynamic initializers in the translation unit. Initializations of entities
+at namespace scope are appearance-ordered. This (recursively) extends to
+imported modules at the point of appearance of the import declaration.

ChuanqiXu9 wrote:

I feel it might not be related here?

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -8,109 +8,91 @@ Standard C++ Modules
 Introduction
 
 
-The term ``modules`` has a lot of meanings. For the users of Clang, modules may
-refer to ``Objective-C Modules``, ``Clang C++ Modules`` (or ``Clang Header 
Modules``,
-etc.) or ``Standard C++ Modules``. The implementation of all these kinds of 
modules in Clang
-has a lot of shared code, but from the perspective of users, their semantics 
and
-command line interfaces are very different. This document focuses on
-an introduction of how to use standard C++ modules in Clang.
-
-There is already a detailed document about `Clang modules `_, it
-should be helpful to read `Clang modules `_ if you want to know
-more about the general idea of modules. Since standard C++ modules have 
different semantics
-(and work flows) from `Clang modules`, this page describes the background and 
use of
-Clang with standard C++ modules.
-
-Modules exist in two forms in the C++ Language Specification. They can refer to
-either "Named Modules" or to "Header Units". This document covers both forms.
+The term ``module`` has a lot of meanings. For Clang users, a module may refer
+to an ``Objective-C Module``, `Clang Module `_ (also called a
+``Clang Header Module``) or a ``C++20 Module`` (or a ``Standard C++ Module``).
+The implementation of all these kinds of modules in Clang shares a lot of code,
+but from the perspective of users, their semantics and command line interfaces
+are very different. This document focuses on an introduction to the use of
+C++20 modules in Clang. In the remainder of this document, the term ``module``
+will refer to Standard C++20 modules and the term ``Clang module`` will refer
+to the Clang modules extension.
+
+Modules exist in two forms in the C++ Standard. They can refer to either
+"Named Modules" or "Header Units". This document covers both forms.
 
 Standard C++ Named modules
 ==
 
-This document was intended to be a manual first and foremost, however, we 
consider it helpful to
-introduce some language background here for readers who are not familiar with
-the new language feature. This document is not intended to be a language
-tutorial; it will only introduce necessary concepts about the
-structure and building of the project.
+In order to understand compiler behavior, it is helpful to introduce some
+terms and definitions for readers who are not familiar with the C++ feature.
+This document is not a tutorial on C++; it only introduces necessary concepts
+to better understand use of modules in a project.
 
 Background and terminology
 --
 
-Modules
-~~~
-
-In this document, the term ``Modules``/``modules`` refers to standard C++ 
modules
-feature if it is not decorated by ``Clang``.
-
-Clang Modules
-~
-
-In this document, the term ``Clang Modules``/``Clang modules`` refer to Clang
-c++ modules extension. These are also known as ``Clang header modules``,
-``Clang module map modules`` or ``Clang c++ modules``.
-
 Module and module unit
 ~~
 
-A module consists of one or more module units. A module unit is a special
-translation unit. Every module unit must have a module declaration. The syntax
-of the module declaration is:
+A module consists of one or more module units. A module unit is a special kind
+of translation unit. Every module unit must have a module declaration. The
+syntax of the module declaration is:
 
 .. code-block:: c++
 
   [export] module module_name[:partition_name];
 
-Terms enclosed in ``[]`` are optional. The syntax of ``module_name`` and 
``partition_name``
-in regex form corresponds to ``[a-zA-Z_][a-zA-Z_0-9\.]*``. In particular, a 
literal dot ``.``
-in the name has no semantic meaning (e.g. implying a hierarchy).
+Terms enclosed in ``[]`` are optional. ``module_name`` and ``partition_name``
+are typical C++ identifiers, except that they may contain a period (``.``).
+Note that a ``.`` in the name has no semantic meaning (e.g. implying a
+hierarchy or referring to the file system).
 
-In this document, module units are classified into:
+In this document, module units are classified as:
 
-* Primary module interface unit.
-
-* Module implementation unit.
-
-* Module interface partition unit.
-
-* Internal module partition unit.
+* Primary module interface unit
+* Module implementation unit
+* Module partition interface unit
+* Module partition implementation unit
 
 A primary module interface unit is a module unit whose module declaration is
-``export module module_name;``. The ``module_name`` here denotes the name of 
the
+``export module module_name;`` where ``module_name`` denotes the name of the
 module. A module should have one and only one primary module interface unit.
 
 A module implementation unit is a module unit whose module declaration is
-``module module_name;``. A module could have multiple module implementation
-units with the same declaration.
+``module module_name;``. Multiple module 

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -400,24 +389,27 @@ And the compilation process for module units are like:
 mod1.cppm -> clang++ mod1.cppm ... -> mod1.pcm --,--> clang++ 
mod1.pcm ... -> mod1.o -+
 src2.cpp +> clang++ 
src2.cpp ---> src2.o -'
 
-As the diagrams show, we need to compile the BMI from module units to object 
files and link the object files.
-(But we can't do this for the BMI from header units. See the later section for 
the definition of header units)
+As the diagrams show, we need to compile the BMI from module units to object
+files and then link the object files. (However, we can't do this for the BMI
+from header units. See the section on :ref:`header units ` for
+more details.
 
-If we want to create a module library, we can't just ship the BMIs in an 
archive.
-We must compile these BMIs(``*.pcm``) into object files(``*.o``) and add those 
object files to the archive instead.
+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.
 
-Consistency Requirement
-~~~
+Consistency Requirements
+
 
-If we envision modules as a cache to speed up compilation, then - as with 
other caching techniques -
-it is important to keep cache consistency.
-So **currently** Clang will do very strict check for consistency.
+If modules are thought of as a kind of cache to speed up compilation, then, as
+with other caching techniques, it is important to keep cache consistency. Clang
+does very strict checking for that.
 
 Options consistency
 ^^^
 
-The language option of module units and their non-module-unit users should be 
consistent.
-The following example is not allowed:
+Language dialect compiler options for module units and their non-module-unit

ChuanqiXu9 wrote:

This is my first time to see the term `Language dialect compiler options`. 
Maybe it is better to explain the language option as compiler options may 
affect the semantics of the program if the term "language option" is not clear?



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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -216,51 +198,56 @@ We explain the options in the following sections.
 How to enable standard C++ modules
 ~~
 
-Currently, standard C++ modules are enabled automatically
-if the language standard is ``-std=c++20`` or newer.
+Standard C++ modules are enabled automatically if the language standard is
+``-std=c++20`` or newer.
 
 How to produce a BMI
 
 
-We can generate a BMI for an importable module unit by either ``--precompile``
-or ``-fmodule-output`` flags.
+To generate a BMI for an importable module unit, use either the 
``--precompile``
+or ``-fmodule-output`` command line option.
 
-The ``--precompile`` option generates the BMI as the output of the compilation 
and the output path
-can be specified using the ``-o`` option.
+The ``--precompile`` option generates the BMI as the output of the compilation
+and the output path can be specified using the ``-o`` option.
 
-The ``-fmodule-output`` option generates the BMI as a by-product of the 
compilation.
-If ``-fmodule-output=`` is specified, the BMI will be emitted the specified 
location. Then if
-``-fmodule-output`` and ``-c`` are specified, the BMI will be emitted in the 
directory of the
-output file with the name of the input file with the new extension ``.pcm``. 
Otherwise, the BMI
-will be emitted in the working directory with the name of the input file with 
the new extension
+The ``-fmodule-output`` option generates the BMI as a by-product of the
+compilation. If ``-fmodule-output=`` is specified, the BMI will be emitted to
+the specified location. If ``-fmodule-output`` and ``-c`` are specified, the
+BMI will be emitted in the directory of the output file with the name of the
+input file with the extension ``.pcm``. Otherwise, the BMI will be emitted in
+the working directory with the name of the input file with the extension

ChuanqiXu9 wrote:

e.g,

```
clang++ a.cpp -c -o result/a.o
```

then `.` is the `working directory` and `./result` is the directory of the 
output file.

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -8,109 +8,91 @@ Standard C++ Modules
 Introduction
 
 
-The term ``modules`` has a lot of meanings. For the users of Clang, modules may
-refer to ``Objective-C Modules``, ``Clang C++ Modules`` (or ``Clang Header 
Modules``,
-etc.) or ``Standard C++ Modules``. The implementation of all these kinds of 
modules in Clang
-has a lot of shared code, but from the perspective of users, their semantics 
and
-command line interfaces are very different. This document focuses on
-an introduction of how to use standard C++ modules in Clang.
-
-There is already a detailed document about `Clang modules `_, it
-should be helpful to read `Clang modules `_ if you want to know
-more about the general idea of modules. Since standard C++ modules have 
different semantics
-(and work flows) from `Clang modules`, this page describes the background and 
use of
-Clang with standard C++ modules.
-
-Modules exist in two forms in the C++ Language Specification. They can refer to
-either "Named Modules" or to "Header Units". This document covers both forms.
+The term ``module`` has a lot of meanings. For Clang users, a module may refer
+to an ``Objective-C Module``, `Clang Module `_ (also called a
+``Clang Header Module``) or a ``C++20 Module`` (or a ``Standard C++ Module``).
+The implementation of all these kinds of modules in Clang shares a lot of code,
+but from the perspective of users, their semantics and command line interfaces
+are very different. This document focuses on an introduction to the use of
+C++20 modules in Clang. In the remainder of this document, the term ``module``
+will refer to Standard C++20 modules and the term ``Clang module`` will refer
+to the Clang modules extension.
+
+Modules exist in two forms in the C++ Standard. They can refer to either
+"Named Modules" or "Header Units". This document covers both forms.
 
 Standard C++ Named modules
 ==
 
-This document was intended to be a manual first and foremost, however, we 
consider it helpful to
-introduce some language background here for readers who are not familiar with
-the new language feature. This document is not intended to be a language
-tutorial; it will only introduce necessary concepts about the
-structure and building of the project.
+In order to understand compiler behavior, it is helpful to introduce some
+terms and definitions for readers who are not familiar with the C++ feature.
+This document is not a tutorial on C++; it only introduces necessary concepts
+to better understand use of modules in a project.
 
 Background and terminology
 --
 
-Modules
-~~~
-
-In this document, the term ``Modules``/``modules`` refers to standard C++ 
modules
-feature if it is not decorated by ``Clang``.
-
-Clang Modules
-~
-
-In this document, the term ``Clang Modules``/``Clang modules`` refer to Clang
-c++ modules extension. These are also known as ``Clang header modules``,
-``Clang module map modules`` or ``Clang c++ modules``.
-
 Module and module unit
 ~~
 
-A module consists of one or more module units. A module unit is a special
-translation unit. Every module unit must have a module declaration. The syntax
-of the module declaration is:
+A module consists of one or more module units. A module unit is a special kind
+of translation unit. Every module unit must have a module declaration. The
+syntax of the module declaration is:
 
 .. code-block:: c++
 
   [export] module module_name[:partition_name];
 
-Terms enclosed in ``[]`` are optional. The syntax of ``module_name`` and 
``partition_name``
-in regex form corresponds to ``[a-zA-Z_][a-zA-Z_0-9\.]*``. In particular, a 
literal dot ``.``
-in the name has no semantic meaning (e.g. implying a hierarchy).
+Terms enclosed in ``[]`` are optional. ``module_name`` and ``partition_name``
+are typical C++ identifiers, except that they may contain a period (``.``).
+Note that a ``.`` in the name has no semantic meaning (e.g. implying a
+hierarchy or referring to the file system).
 
-In this document, module units are classified into:
+In this document, module units are classified as:
 
-* Primary module interface unit.
-
-* Module implementation unit.
-
-* Module interface partition unit.
-
-* Internal module partition unit.
+* Primary module interface unit
+* Module implementation unit
+* Module partition interface unit
+* Module partition implementation unit
 
 A primary module interface unit is a module unit whose module declaration is
-``export module module_name;``. The ``module_name`` here denotes the name of 
the
+``export module module_name;`` where ``module_name`` denotes the name of the
 module. A module should have one and only one primary module interface unit.
 
 A module implementation unit is a module unit whose module declaration is
-``module module_name;``. A module could have multiple module implementation
-units with the same declaration.
+``module module_name;``. Multiple module 

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -925,45 +923,41 @@ In that case, you need to convert your source files (.cpp 
files) to module imple
   // Following off should be unchanged.
   ...
 
-The module implementation unit will import the primary module implicitly.
-We don't include any headers in the module implementation units
-here since we want to avoid duplicated declarations between translation units.
-This is the reason why we add non-exported using declarations from the third
-party libraries in the primary module interface unit.
+The module implementation unit will import the primary module implicitly. Do
+not include any headers in the module implementation units because that avoids
+duplicated declarations between translation units. This is why non-exported
+using declarations are added from third-party libraries in the primary module
+interface unit.
 
-And if you provide your library as ``libyour_library.so``, you probably need to
-provide a modular one ``libyour_library_modules.so`` since you changed the ABI.
+If the library is provided as ``libyour_library.so``, a modular library (e.g.,
+``libyour_library_modules.so``) may also need to be provided for ABI
+compatibility.
 
 What if there are headers only inclued by the source files
 ^^
 
-The above practice may be problematic if there are headers only included by 
the source
-files. If you're using private module fragment, you may solve the issue by 
including them
-in the private module fragment. While it is OK to solve it by including the 
implementation
-headers in the module purview if you're using implementation module units, it 
may be
-suboptimal since the primary module interface units now containing entities 
not belongs
-to the interface.
-
-If you're a perfectionist, maybe you can improve it by introducing internal 
module partition unit.
+The above practice may be problematic if there are headers only included by the
+source files. When using a private module fragment, this issue may be solved by
+including those headers in the private module fragment. While it is OK to solve
+it by including the implementation headers in the module purview when using
+implementation module units, it may be suboptimal because the primary module
+interface units now contain entities that do not belong to the interface.
 
-The internal module partition unit is an importable module unit which is 
internal
-to the module itself. The concept just meets the headers only included by the 
source files.
-
-We don't show code snippet since it may be too verbose or not good or not 
general.
-But it may not be too hard if you can understand the points of the section.
+This can potentially be improved by introducing module partition implementation
+unit. The module partition implementation unit is an importable module unit
+which is internal to the module itself. However, this approach may not always
+be the best way forward.

ChuanqiXu9 wrote:

Maybe I misunderstand the sentence "However, this approach may not always
be the best way forward." But it reads as, it is not good to use `module 
partition implementation unit`. This is not true.

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -312,75 +300,76 @@ So all of the following name is not valid by default:
 __test
 // and so on ...
 
-If you still want to use the reserved module names for any reason, use
-``-Wno-reserved-module-identifier`` to suppress the warning.
+Using a reserved module name is strongly discouraged, but
+``-Wno-reserved-module-identifier`` can be used to suppress the warning.
 
-How to specify the dependent BMIs
-~
+Specifying dependent BMIs
+~
 
-There are 3 methods to specify the dependent BMIs:
+There are 3 ways to specify a dependent BMI:
 
-* (1) ``-fprebuilt-module-path=``.
-* (2) ``-fmodule-file=`` (Deprecated).
-* (3) ``-fmodule-file==``.
+1. ``-fprebuilt-module-path=``.
+2. ``-fmodule-file=`` (Deprecated).
+3. ``-fmodule-file==``.
 
-The option ``-fprebuilt-module-path`` tells the compiler the path where to 
search for dependent BMIs.
-It may be used multiple times just like ``-I`` for specifying paths for header 
files. The look up rule here is:
+The ``-fprebuilt-module-path`` option specifies the path to search for
+dependent BMIs. Multiple paths may be specified, similar to using ``-I`` to
+specify a search path for header files. When importing a module ``M``, the
+compiler looks for ``M.pcm`` in the directories specified by
+``-fprebuilt-module-path``. Similarly,  When importing a partition module unit
+``M:P``, the compiler looks for ``M-P.pcm`` in the directories specified by
+``-fprebuilt-module-path``.
 
-* (1) When we import module M. The compiler would look up M.pcm in the 
directories specified
-  by ``-fprebuilt-module-path``.
-* (2) When we import partition module unit M:P. The compiler would look up 
M-P.pcm in the
-  directories specified by ``-fprebuilt-module-path``.
-
-The option ``-fmodule-file=`` tells the compiler to load the 
specified BMI directly.
-The option ``-fmodule-file==`` tells the compiler to 
load the specified BMI
-for the module specified by  when necessary. The main 
difference is that
+The ``-fmodule-file=`` option causes the compiler to load the
+specified BMI directly. The ``-fmodule-file==``
+option causes the compiler to load the specified BMI for the module specified
+by  when necessary. The main difference is that
 ``-fmodule-file=`` will load the BMI eagerly, whereas
-``-fmodule-file==`` will only load the BMI lazily, 
which is similar
-with ``-fprebuilt-module-path``. The option ``-fmodule-file=`` 
for named modules is deprecated
-and is planning to be removed in future versions.
+``-fmodule-file==`` will only load the BMI lazily,
+which is similar to ``-fprebuilt-module-path``. The
+``-fmodule-file=`` option for named modules is deprecated and will
+be removed in a future version of Clang.
 
-In case all ``-fprebuilt-module-path=``, 
``-fmodule-file=`` and
-``-fmodule-file==`` exist, the 
``-fmodule-file=`` option
-takes highest precedence and ``-fmodule-file==`` 
will take the second
-highest precedence.
+When these options are specified in the same invocation of the compiler, the
+``-fmodule-file=`` option takes precedence over
+``-fmodule-file==``, which takes precedence over
+``-fprebuilt-module-path=``.
 
-We need to specify all the dependent (directly and indirectly) BMIs.
-See https://github.com/llvm/llvm-project/issues/62707 for detail.
+Note: you must specify all the (directly or indirectly) dependent BMIs
+explicitly. See https://github.com/llvm/llvm-project/issues/62707 for details.
 
-When we compile a ``module implementation unit``, we must specify the BMI of 
the corresponding
-``primary module interface unit``.
-Since the language specification says a module implementation unit implicitly 
imports
-the primary module interface unit.
+When compiling a ``module implementation unit``, the BMI of the corresponding
+``primary module interface unit`` must be specified. This is because a module
+implementation unit implicitly imports the primary module interface unit.
 
   [module.unit]p8
 
   A module-declaration that contains neither an export-keyword nor a 
module-partition implicitly
   imports the primary module interface unit of the module as if by a 
module-import-declaration.
 
-All of the 3 options ``-fprebuilt-module-path=``, 
``-fmodule-file=``
-and ``-fmodule-file==`` may occur multiple times.
-For example, the command line to compile ``M.cppm`` in
-the above example could be rewritten into:
+The ``-fprebuilt-module-path=``, 
``-fmodule-file=``, 
+and ``-fmodule-file==`` options may be specified
+multiple times. For example, the command line to compile ``M.cppm`` in
+the previous example could be rewritten as:
 
 .. code-block:: console
 
   $ clang++ -std=c++20 M.cppm --precompile 
-fmodule-file=M:interface_part=M-interface_part.pcm 
-fmodule-file=M:impl_part=M-impl_part.pcm -o M.pcm
 
 When there are multiple ``-fmodule-file==`` options for the same
-, the last ``-fmodule-file==`` will override the 
previous
-``-fmodule-file==`` options.
+, t

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -8,79 +8,60 @@ Standard C++ Modules
 Introduction
 
 
-The term ``modules`` has a lot of meanings. For the users of Clang, modules may
-refer to ``Objective-C Modules``, ``Clang C++ Modules`` (or ``Clang Header 
Modules``,
-etc.) or ``Standard C++ Modules``. The implementation of all these kinds of 
modules in Clang
-has a lot of shared code, but from the perspective of users, their semantics 
and
-command line interfaces are very different. This document focuses on
-an introduction of how to use standard C++ modules in Clang.
-
-There is already a detailed document about `Clang modules `_, it
-should be helpful to read `Clang modules `_ if you want to know
-more about the general idea of modules. Since standard C++ modules have 
different semantics
-(and work flows) from `Clang modules`, this page describes the background and 
use of
-Clang with standard C++ modules.
-
-Modules exist in two forms in the C++ Language Specification. They can refer to
-either "Named Modules" or to "Header Units". This document covers both forms.
+The term ``modules`` has a lot of meanings. For Clang users, modules may refer
+to ``Objective-C Modules``, `Clang Modules `_ (also called
+``Clang Header Modules``, etc.) or ``C++20 Modules`` (or
+``Standard C++ Modules``). The implementation of all these kinds of modules in
+Clang shares a lot of code, but from the perspective of users, their semantics
+and command line interfaces are very different. This document focuses on an
+introduction of focusing on the use of C++20 modules in Clang. In the remainder
+of this document, the term ``modules`` will refer to Standard C++20 modules and
+the term ``Clang modules`` will refer to the Clang modules extension.
+
+Modules exist in two forms in the C++ Standard. They can refer to either
+"Named Modules" or "Header Units". This document covers both forms.
 
 Standard C++ Named modules
 ==
 
-This document was intended to be a manual first and foremost, however, we 
consider it helpful to
-introduce some language background here for readers who are not familiar with
-the new language feature. This document is not intended to be a language
-tutorial; it will only introduce necessary concepts about the
-structure and building of the project.
+In order to understand compiler behavior, it is helpful to introduce some
+language background here for readers who are not familiar with the C++ feature.
+This document is not a tutorial on C++; it only introduces necessary concepts
+to better understand use of modules for a project.
 
 Background and terminology
 --
 
-Modules
-~~~
-
-In this document, the term ``Modules``/``modules`` refers to standard C++ 
modules
-feature if it is not decorated by ``Clang``.
-
-Clang Modules
-~
-
-In this document, the term ``Clang Modules``/``Clang modules`` refer to Clang
-c++ modules extension. These are also known as ``Clang header modules``,
-``Clang module map modules`` or ``Clang c++ modules``.
-
 Module and module unit
 ~~
 
-A module consists of one or more module units. A module unit is a special
-translation unit. Every module unit must have a module declaration. The syntax
-of the module declaration is:
+A module consists of one or more module units. A module unit is a special kind
+of translation unit. Every module unit must have a module declaration. The
+syntax of the module declaration is:
 
 .. code-block:: c++
 
   [export] module module_name[:partition_name];
 
-Terms enclosed in ``[]`` are optional. The syntax of ``module_name`` and 
``partition_name``
-in regex form corresponds to ``[a-zA-Z_][a-zA-Z_0-9\.]*``. In particular, a 
literal dot ``.``
-in the name has no semantic meaning (e.g. implying a hierarchy).
-
-In this document, module units are classified into:
-
-* Primary module interface unit.
-
-* Module implementation unit.
+Terms enclosed in ``[]`` are optional. ``module_name`` and ``partition_name``
+are typical C++ identifiers, except that they may contain a period (``.``).

ChuanqiXu9 wrote:

Yes

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

https://github.com/ChuanqiXu9 commented:

Big thanks!

I left some comments about correctness or clearness. And all other change looks 
good to me.

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -738,22 +736,21 @@ the following style significantly:
   import M;
   ... // use declarations from module M.
 
-The key part of the tip is to reduce the duplications from the text includes.
+Reducing the duplication from textual includes is what improves compile-time
+performance.
 
-Ideas for converting to modules

+Transitioning to modules
+
 
-For new libraries, we encourage them to use modules completely from day one if 
possible.
-This will be pretty helpful to make the whole ecosystems to get ready.
+New code and libraries should use modules from the start if possible. However,

ChuanqiXu9 wrote:

I am not sure if `should` is a too strong term from the non-native speaker 
perspective.

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -8,79 +8,60 @@ Standard C++ Modules
 Introduction
 
 
-The term ``modules`` has a lot of meanings. For the users of Clang, modules may
-refer to ``Objective-C Modules``, ``Clang C++ Modules`` (or ``Clang Header 
Modules``,
-etc.) or ``Standard C++ Modules``. The implementation of all these kinds of 
modules in Clang
-has a lot of shared code, but from the perspective of users, their semantics 
and
-command line interfaces are very different. This document focuses on
-an introduction of how to use standard C++ modules in Clang.
-
-There is already a detailed document about `Clang modules `_, it
-should be helpful to read `Clang modules `_ if you want to know
-more about the general idea of modules. Since standard C++ modules have 
different semantics
-(and work flows) from `Clang modules`, this page describes the background and 
use of
-Clang with standard C++ modules.
-
-Modules exist in two forms in the C++ Language Specification. They can refer to
-either "Named Modules" or to "Header Units". This document covers both forms.
+The term ``modules`` has a lot of meanings. For Clang users, modules may refer
+to ``Objective-C Modules``, `Clang Modules `_ (also called
+``Clang Header Modules``, etc.) or ``C++20 Modules`` (or
+``Standard C++ Modules``). The implementation of all these kinds of modules in
+Clang shares a lot of code, but from the perspective of users, their semantics
+and command line interfaces are very different. This document focuses on an
+introduction of focusing on the use of C++20 modules in Clang. In the remainder
+of this document, the term ``modules`` will refer to Standard C++20 modules and
+the term ``Clang modules`` will refer to the Clang modules extension.
+
+Modules exist in two forms in the C++ Standard. They can refer to either
+"Named Modules" or "Header Units". This document covers both forms.
 
 Standard C++ Named modules
 ==
 
-This document was intended to be a manual first and foremost, however, we 
consider it helpful to
-introduce some language background here for readers who are not familiar with
-the new language feature. This document is not intended to be a language
-tutorial; it will only introduce necessary concepts about the
-structure and building of the project.
+In order to understand compiler behavior, it is helpful to introduce some
+language background here for readers who are not familiar with the C++ feature.
+This document is not a tutorial on C++; it only introduces necessary concepts
+to better understand use of modules for a project.
 
 Background and terminology
 --
 
-Modules
-~~~
-
-In this document, the term ``Modules``/``modules`` refers to standard C++ 
modules
-feature if it is not decorated by ``Clang``.
-
-Clang Modules
-~
-
-In this document, the term ``Clang Modules``/``Clang modules`` refer to Clang
-c++ modules extension. These are also known as ``Clang header modules``,
-``Clang module map modules`` or ``Clang c++ modules``.
-
 Module and module unit
 ~~
 
-A module consists of one or more module units. A module unit is a special
-translation unit. Every module unit must have a module declaration. The syntax
-of the module declaration is:
+A module consists of one or more module units. A module unit is a special kind
+of translation unit. Every module unit must have a module declaration. The

ChuanqiXu9 wrote:

Technically not true: https://eel.is/c++draft/gram.basic#:translation-unit.

More specifically, a valid module unit may be:

```
module;
#include 
export module M;
...
```

Here the first `module;` keywords is not considered as module declaration.

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -577,15 +574,16 @@ the generated BMI specified by ``-o`` will be full BMI 
and the BMI specified by
-> ...
-> consumer_n.cpp
 
-We don't emit diagnostics if ``-fexperimental-modules-reduced-bmi`` is used 
with a non-module
-unit. This design helps the end users of one phase compilation model to 
perform experiments
-early without asking for the help of build systems. The users of build systems 
which supports
-two phase compilation model still need helps from build systems.
+Clang does not emit diagnostics when ``-fexperimental-modules-reduced-bmi`` is
+used with a non-module unit. This design helps the end users of the one-phase
+compilation model to perform experiments without needing to modify the build

ChuanqiXu9 wrote:

Or try Reduced BMI?

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

[clang] Revise the modules document for clarity (PR #90237)

[Chuanqi Xu via cfe-commits]

@@ -530,43 +527,43 @@ Now the linkage name of ``NS::foo()`` will be 
``_ZN2NS3fooEv``.
 Module Initializers
 ~~~
 
-All the importable module units are required to emit an initializer function.
-The initializer function should contain calls to importing modules first and
-all the dynamic-initializers in the current module unit then.
-
-Translation units explicitly or implicitly importing named modules must call
-the initializer functions of the imported named modules within the sequence of
-the dynamic-initializers in the TU. Initializations of entities at namespace
-scope are appearance-ordered. This (recursively) extends into imported modules
-at the point of appearance of the import declaration.
+All importable module units are required to emit an initializer function. The
+initializer function emits calls to imported modules first followed by calls
+to all to dynamic initializers in the current module unit.
 
-It is allowed to omit calls to importing modules if it is known empty.
+Translation units that explicitly or implicitly import a named module must call
+the initializer functions of the imported named module within the sequence of
+the dynamic initializers in the translation unit. Initializations of entities
+at namespace scope are appearance-ordered. This (recursively) extends to
+imported modules at the point of appearance of the import declaration.
 
-It is allowed to omit calls to importing modules for which is known to be 
called.
+If the imported module is known to be empty, the call to its initializer may be
+omitted. Additionally, if the imported module is known to have already been
+imported, the call to its initializer may be omitted.
 
 Reduced BMI
 ---
 
-To support the 2 phase compilation model, Clang chose to put everything needed 
to
-produce an object into the BMI. But every consumer of the BMI, except itself, 
doesn't
-need such informations. It makes the BMI to larger and so may introduce 
unnecessary
-dependencies into the BMI. To mitigate the problem, we decided to reduce the 
information
-contained in the BMI.
+To support the two-phase compilation model, Clang puts everything needed to
+produce an object into the BMI. However, other consumers of the BMI generally
+don't need that informations. This makes the BMI larger and may introduce
+unnecessary dependencies for the BMI. To mitigate the problem, Clang added a
+compiler option to reduce the information contained in the BMI. These two
+formats are known as Full BMI and Reduced BMI, respectively.
 
-To be clear, we call the default BMI as Full BMI and the new introduced BMI as 
Reduced
-BMI.
+Users can use the ``-fexperimental-modules-reduced-bmi`` option to produce a
+Reduced BMI.

ChuanqiXu9 wrote:

Do you mean `Reduced BMI`? The name was discussed in 
`https://discourse.llvm.org/t/rfc-c-20-modules-introduce-thin-bmi-and-decls-hash/74755/52`.

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

[clang] 487967a - [Modules] Don't replace local declarations with external declaration with lower visibility

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-04-28T15:23:39+08:00
New Revision: 487967af82053cd08022635a2ff768385d936c80

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

LOG: [Modules] Don't replace local declarations with external declaration with 
lower visibility

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

For the reproducer:

```
//--- header.h

namespace N {
template
concept X = true;

template
class Y {
public:
template
friend class Y;
};

inline Y x;
}

//--- bar.cppm
module;
export module bar;
namespace N {
// To make sure N::Y won't get elided.
using N::x;
}

//--- foo.cc
// expected-no-diagnostics
import bar;
void y() {
N::Y y{};
};
```

it will crash. The root cause is that in
`StoredDeclsList::replaceExternalDecls`, we will replace the
existing declarations with external declarations.

Then for the reproducer, the redecl chain for Y is like:

```
Y (Local) -> Y (Local, friend) -> Y (Imported) -> Y(Imported, friend)
```

Before the lookup, the stored lookup result is `Y(Local)` then we find
`Y(Imported)`. And now we repalce `Y(Local)` with `Y(Imported)`. But
`Y(Imported)` is not visible. So we tried to find if there is any
redeclarations visible but we find `Y(Local, friend)`, then problem
happens.

The solution is try to avoid the replace to happen if the external
declaration has lower visibility then we can always find the local
declarations. This may help the lookup performance slightly.

Also I found the implementation of
`StoredDeclsList::replaceExternalDecls` is not efficiency. It has an
`O(n*m)` complexities. But let's improve that in the future.

Added: 
clang/test/Modules/pr88400.cppm

Modified: 
clang/include/clang/AST/DeclContextInternals.h

Removed: 




diff  --git a/clang/include/clang/AST/DeclContextInternals.h 
b/clang/include/clang/AST/DeclContextInternals.h
index c4734ab5789538..42cc677f82135e 100644
--- a/clang/include/clang/AST/DeclContextInternals.h
+++ b/clang/include/clang/AST/DeclContextInternals.h
@@ -160,12 +160,16 @@ class StoredDeclsList {
 
   void replaceExternalDecls(ArrayRef Decls) {
 // Remove all declarations that are either external or are replaced with
-// external declarations.
+// external declarations with higher visibilities.
 erase_if([Decls](NamedDecl *ND) {
   if (ND->isFromASTFile())
 return true;
+  // FIXME: Can we get rid of this loop completely?
   for (NamedDecl *D : Decls)
-if (D->declarationReplaces(ND, /*IsKnownNewer=*/false))
+// Only replace the local declaration if the external declaration has
+// higher visibilities.
+if (D->getModuleOwnershipKind() <= ND->getModuleOwnershipKind() &&
+D->declarationReplaces(ND, /*IsKnownNewer=*/false))
   return true;
   return false;
 });

diff  --git a/clang/test/Modules/pr88400.cppm b/clang/test/Modules/pr88400.cppm
new file mode 100644
index 00..ff69137a0b9040
--- /dev/null
+++ b/clang/test/Modules/pr88400.cppm
@@ -0,0 +1,61 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/bar.cppm -emit-module-interface -o %t/bar.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo.cc -fmodule-file=bar=%t/bar.pcm 
-fsyntax-only -verify
+// RUN: %clang_cc1 -std=c++20 %t/bar.cc -fmodule-file=bar=%t/bar.pcm 
-fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/bar.cppm -emit-reduced-module-interface -o 
%t/bar.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo.cc -fmodule-file=bar=%t/bar.pcm 
-fsyntax-only -verify
+// RUN: %clang_cc1 -std=c++20 %t/bar.cc -fmodule-file=bar=%t/bar.pcm 
-fsyntax-only -verify
+
+//--- header.h
+#pragma once
+
+namespace N {
+template
+concept X = true;
+
+template
+class Y {
+public:
+template
+friend class Y;
+};
+
+inline Y x;
+}
+
+//--- bar.cppm
+module;
+
+#include "header.h"
+
+export module bar;
+
+namespace N {
+// To make sure N::Y won't get elided.
+using N::x;
+}
+
+//--- foo.cc
+// expected-no-diagnostics
+#include "header.h"
+
+import bar;
+
+void y() {
+N::Y y{};
+};
+
+//--- bar.cc
+// expected-no-diagnostics
+import bar;
+
+#include "header.h"
+
+void y() {
+N::Y y{};
+};
+



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

[clang] 367efa0 - [NFC] [Modules] Avoid scanning the stored decl list twice when replace

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-04-28T18:31:10+08:00
New Revision: 367efa0b0542e6f4171e8c914728946c302ab24b

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

LOG: [NFC] [Modules] Avoid scanning the stored decl list twice when replace
external decls

This patch fixes a FIXME in StoredDeclList::replaceExternalDecls.

StoredDeclList::replaceExternalDecls will iterate the list first to
remove some declarations and iterate the list again to get the tail of
the list.

It should be better to avoid the second iterations.

Added: 


Modified: 
clang/include/clang/AST/DeclContextInternals.h

Removed: 




diff  --git a/clang/include/clang/AST/DeclContextInternals.h 
b/clang/include/clang/AST/DeclContextInternals.h
index 42cc677f82135e..e169c485921929 100644
--- a/clang/include/clang/AST/DeclContextInternals.h
+++ b/clang/include/clang/AST/DeclContextInternals.h
@@ -42,11 +42,12 @@ class StoredDeclsList {
   /// external declarations.
   DeclsAndHasExternalTy Data;
 
-  template
-  void erase_if(Fn ShouldErase) {
+  template  DeclListNode::Decls *erase_if(Fn ShouldErase) {
 Decls List = Data.getPointer();
+
 if (!List)
-  return;
+  return nullptr;
+
 ASTContext &C = getASTContext();
 DeclListNode::Decls NewHead = nullptr;
 DeclListNode::Decls *NewLast = nullptr;
@@ -79,6 +80,17 @@ class StoredDeclsList {
 Data.setPointer(NewHead);
 
 assert(llvm::none_of(getLookupResult(), ShouldErase) && "Still exists!");
+
+if (!Data.getPointer())
+  // All declarations are erased.
+  return nullptr;
+else if (NewHead.is())
+  // The list only contains a declaration, the header itself.
+  return (DeclListNode::Decls *)&Data;
+else {
+  assert(NewLast && NewLast->is() && "Not the tail?");
+  return NewLast;
+}
   }
 
   void erase(NamedDecl *ND) {
@@ -161,7 +173,7 @@ class StoredDeclsList {
   void replaceExternalDecls(ArrayRef Decls) {
 // Remove all declarations that are either external or are replaced with
 // external declarations with higher visibilities.
-erase_if([Decls](NamedDecl *ND) {
+DeclListNode::Decls *Tail = erase_if([Decls](NamedDecl *ND) {
   if (ND->isFromASTFile())
 return true;
   // FIXME: Can we get rid of this loop completely?
@@ -189,24 +201,15 @@ class StoredDeclsList {
   DeclsAsList = Node;
 }
 
-DeclListNode::Decls Head = Data.getPointer();
-if (Head.isNull()) {
+if (!Data.getPointer()) {
   Data.setPointer(DeclsAsList);
   return;
 }
 
-// Find the end of the existing list.
-// FIXME: It would be possible to preserve information from erase_if to
-// avoid this rescan looking for the end of the list.
-DeclListNode::Decls *Tail = &Head;
-while (DeclListNode *Node = Tail->dyn_cast())
-  Tail = &Node->Rest;
-
 // Append the Decls.
 DeclListNode *Node = C.AllocateDeclListNode(Tail->get());
 Node->Rest = DeclsAsList;
 *Tail = Node;
-Data.setPointer(Head);
   }
 
   /// Return the list of all the decls.



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

[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)

[Chuanqi Xu via cfe-commits]

@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
   // Set up the new entry block.
   replaceEntryBlock();
 
+  // Turn symmetric transfers into musttail calls.
+  for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+ResumeCall->setCallingConv(NewF->getCallingConv());
+if (TTI.supportsTailCallFor(ResumeCall)) {
+  // FIXME: Could we support symmetric transfer effectively without
+  // musttail?
+  ResumeCall->setTailCallKind(CallInst::TCK_MustTail);
+}
+
+// Put a 'ret void' after the call, and split any remaining instructions to

ChuanqiXu9 wrote:

> Also, maybe this would become moot if we address 
> https://discourse.llvm.org/t/coro-pre-split-handling-of-the-suspend-edge/75043
>  like @jyknight suggested (i.e. not even have the misleading edge)?

But IIRC, it is still possible that we'll have code inserted between 
`llvm.coro.await.suspend.{.*}` and `llvm.coro.suspend`, which is the problem 
we're discussing.

> Sorry for insisting on this, it's maybe because I got "bitten" before (with 
> the suspend), but what other examples do we have where, silently, 
> instructions don't get executed after a call?

If I read correctly, @zmodem said he'd like to mention this in the doc or check 
it by assertions or verifiers. So it looks consensus to me?

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

[clang] [Coroutines][Test] Specify target triple in coro-elide-thinlto (PR #90549)

[Chuanqi Xu via cfe-commits]

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


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

[clang] 38067c5 - [C++20] [Modules] [Reduced BMI] Avoid force writing static declarations

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-04-30T11:34:34+08:00
New Revision: 38067c50a9459caed2892e38b2ae5026a8bff8e2

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

LOG: [C++20] [Modules] [Reduced BMI] Avoid force writing static declarations
within module purview

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

Technically, the static declarations shouldn't be leaked from the module
interface, otherwise it is an illegal program according to the spec. So
we can get rid of the static declarations from the reduced BMI
technically. Then we can close the above issue.

However, there are too many `static inline` codes in existing headers.
So it will be a pretty big breaking change if we do this globally.

Added: 
clang/test/Modules/pr90259.cppm

Modified: 
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 0408eeb6a95b00..7db60c67d71234 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3205,6 +3205,17 @@ void ASTWriter::WriteType(QualType T) {
 // Declaration Serialization
 
//===--===//
 
+static bool IsInternalDeclFromFileContext(const Decl *D) {
+  auto *ND = dyn_cast(D);
+  if (!ND)
+return false;
+
+  if (!D->getDeclContext()->getRedeclContext()->isFileContext())
+return false;
+
+  return ND->getFormalLinkage() == Linkage::Internal;
+}
+
 /// Write the block containing all of the declaration IDs
 /// lexically declared within the given DeclContext.
 ///
@@ -3225,6 +3236,15 @@ uint64_t 
ASTWriter::WriteDeclContextLexicalBlock(ASTContext &Context,
 if (DoneWritingDeclsAndTypes && !wasDeclEmitted(D))
   continue;
 
+// We don't need to write decls with internal linkage into reduced BMI.
+// If such decls gets emitted due to it get used from inline functions,
+// the program illegal. However, there are too many use of static inline
+// functions in the global module fragment and it will be breaking change
+// to forbid that. So we have to allow to emit such declarations from GMF.
+if (GeneratingReducedBMI && !D->isFromExplicitGlobalModule() &&
+IsInternalDeclFromFileContext(D))
+  continue;
+
 KindDeclPairs.push_back(D->getKind());
 KindDeclPairs.push_back(GetDeclRef(D).get());
   }
@@ -3886,6 +3906,13 @@ class ASTDeclContextNameLookupTrait {
   !Writer.wasDeclEmitted(DeclForLocalLookup))
 continue;
 
+  // Try to avoid writing internal decls to reduced BMI.
+  // See comments in ASTWriter::WriteDeclContextLexicalBlock for details.
+  if (Writer.isGeneratingReducedBMI() &&
+  !DeclForLocalLookup->isFromExplicitGlobalModule() &&
+  IsInternalDeclFromFileContext(DeclForLocalLookup))
+continue;
+
   DeclIDs.push_back(Writer.GetDeclRef(DeclForLocalLookup));
 }
 return std::make_pair(Start, DeclIDs.size());
@@ -4257,6 +4284,12 @@ uint64_t 
ASTWriter::WriteDeclContextVisibleBlock(ASTContext &Context,
 if (DoneWritingDeclsAndTypes && !wasDeclEmitted(ND))
   continue;
 
+// We don't need to force emitting internal decls into reduced BMI.
+// See comments in ASTWriter::WriteDeclContextLexicalBlock for details.
+if (GeneratingReducedBMI && !ND->isFromExplicitGlobalModule() &&
+IsInternalDeclFromFileContext(ND))
+  continue;
+
 GetDeclRef(ND);
   }
 }
@@ -4917,8 +4950,7 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema &SemaRef) 
{
   // is ill-formed. However, in practice, there are a lot of projects
   // uses `static inline` in the headers. So we can't get rid of all
   // static entities in reduced BMI now.
-  if (auto *ND = dyn_cast(D);
-  ND && ND->getFormalLinkage() == Linkage::Internal)
+  if (IsInternalDeclFromFileContext(D))
 continue;
 }
 

diff  --git a/clang/test/Modules/pr90259.cppm b/clang/test/Modules/pr90259.cppm
new file mode 100644
index 00..17786998a2a729
--- /dev/null
+++ b/clang/test/Modules/pr90259.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/mod1.cppm -emit-reduced-module-interface -o 
%t/mod-mod1.pcm
+// RUN: %clang_cc1 -std=c++20 %t/mod.cppm -fprebuilt-module-path=%t  \
+// RUN: -emit-reduced-module-interface -o %t/mod.pcm
+// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -verify 
-fsyntax-only
+
+//--- mod1.cppm
+export module mod:mod1;
+namespace {
+int abc = 43;
+}
+namespace mod {
+static int def = 44;
+}
+export int f() {
+return abc + mod::def;
+}
+
+//--- mod.cppm
+// expected-no-diagn

[clang] 18268ac - [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to generate module file for C++20 modules instead of PCHGenerator

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-04-30T13:30:31+08:00
New Revision: 18268ac0f48d93c2bcddb69732761971669c09ab

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

LOG: [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to generate 
module file for C++20 modules instead of PCHGenerator

Previously we're re-using PCHGenerator to generate the module file for
C++20 modules. But this is slighty more or less odd. This patch tries
to use a new class 'CXX20ModulesGenerator' to generate the module file
for C++20 modules.

Added: 


Modified: 
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Serialization/GeneratePCH.cpp
clang/test/Modules/pr67893.cppm
clang/test/Modules/search-partitions.cpp

Removed: 




diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index 6c45b7348b8552..4e433deaaf2dbc 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -928,17 +928,30 @@ class PCHGenerator : public SemaConsumer {
   bool hasEmittedPCH() const { return Buffer->IsComplete; }
 };
 
-class ReducedBMIGenerator : public PCHGenerator {
+class CXX20ModulesGenerator : public PCHGenerator {
 protected:
   virtual Module *getEmittingModule(ASTContext &Ctx) override;
 
+  CXX20ModulesGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
+StringRef OutputFile, bool GeneratingReducedBMI);
+
 public:
-  ReducedBMIGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
-  StringRef OutputFile);
+  CXX20ModulesGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
+StringRef OutputFile)
+  : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
+  /*GeneratingReducedBMI=*/false) {}
 
   void HandleTranslationUnit(ASTContext &Ctx) override;
 };
 
+class ReducedBMIGenerator : public CXX20ModulesGenerator {
+public:
+  ReducedBMIGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
+  StringRef OutputFile)
+  : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
+  /*GeneratingReducedBMI=*/true) {}
+};
+
 /// If we can elide the definition of \param D in reduced BMI.
 ///
 /// Generally, we can elide the definition of a declaration if it won't affect

diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index 04eb1041326713..454653a31534cd 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -272,14 +272,10 @@ bool GenerateModuleInterfaceAction::BeginSourceFileAction(
 std::unique_ptr
 GenerateModuleInterfaceAction::CreateASTConsumer(CompilerInstance &CI,
  StringRef InFile) {
-  CI.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true;
-  CI.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
-  CI.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings = true;
-
-  std::vector> Consumers =
-  CreateMultiplexConsumer(CI, InFile);
-  if (Consumers.empty())
-return nullptr;
+  std::vector> Consumers;
+  Consumers.push_back(std::make_unique(
+  CI.getPreprocessor(), CI.getModuleCache(),
+  CI.getFrontendOpts().OutputFile));
 
   if (CI.getFrontendOpts().GenReducedBMI &&
   !CI.getFrontendOpts().ModuleOutputPath.empty()) {

diff  --git a/clang/lib/Serialization/GeneratePCH.cpp 
b/clang/lib/Serialization/GeneratePCH.cpp
index bed74399098d7f..7b97b73f7bbd00 100644
--- a/clang/lib/Serialization/GeneratePCH.cpp
+++ b/clang/lib/Serialization/GeneratePCH.cpp
@@ -88,31 +88,28 @@ ASTDeserializationListener 
*PCHGenerator::GetASTDeserializationListener() {
   return &Writer;
 }
 
-ReducedBMIGenerator::ReducedBMIGenerator(Preprocessor &PP,
- InMemoryModuleCache &ModuleCache,
- StringRef OutputFile)
+CXX20ModulesGenerator::CXX20ModulesGenerator(Preprocessor &PP,
+ InMemoryModuleCache &ModuleCache,
+ StringRef OutputFile,
+ bool GeneratingReducedBMI)
 : PCHGenerator(
   PP, ModuleCache, OutputFile, llvm::StringRef(),
   std::make_shared(),
   /*Extensions=*/ArrayRef>(),
   /*AllowASTWithErrors*/ false, /*IncludeTimestamps=*/false,
   /*BuildingImplicitModule=*/false, /*ShouldCacheASTInMemory=*/false,
-  /*GeneratingReducedBMI=*/true) {}
+  GeneratingReducedBMI) {}
 
-Module *ReducedBMIGenerator::getEmittingModule(ASTContext &Ctx) {
+Module *C

[clang] fb21343 - [C++20] [Modules] Don't skip pragma diagnostic mappings

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-04-30T13:50:20+08:00
New Revision: fb21343473e33e9a886b42d2fe95d1cec1cd0030

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

LOG: [C++20] [Modules] Don't skip pragma diagnostic mappings

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

Previously, I thought the diagnostic mappings is not meaningful with
modules incorrectly. And this problem get revealed by another change
recently. So this patch tried to rever the previous "optimization"
partially.

Added: 
clang/test/Modules/pr75057.cppm

Modified: 
clang/lib/Serialization/GeneratePCH.cpp

Removed: 




diff  --git a/clang/lib/Serialization/GeneratePCH.cpp 
b/clang/lib/Serialization/GeneratePCH.cpp
index 7b97b73f7bbd00..53dda5f9a38580 100644
--- a/clang/lib/Serialization/GeneratePCH.cpp
+++ b/clang/lib/Serialization/GeneratePCH.cpp
@@ -114,7 +114,6 @@ void 
CXX20ModulesGenerator::HandleTranslationUnit(ASTContext &Ctx) {
   getPreprocessor().getHeaderSearchInfo().getHeaderSearchOpts();
   HSOpts.ModulesSkipDiagnosticOptions = true;
   HSOpts.ModulesSkipHeaderSearchPaths = true;
-  HSOpts.ModulesSkipPragmaDiagnosticMappings = true;
 
   PCHGenerator::HandleTranslationUnit(Ctx);
 

diff  --git a/clang/test/Modules/pr75057.cppm b/clang/test/Modules/pr75057.cppm
new file mode 100644
index 00..374c324e9f495b
--- /dev/null
+++ b/clang/test/Modules/pr75057.cppm
@@ -0,0 +1,62 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// Treat the behavior of using headers as baseline.
+// RUN: %clang_cc1 -std=c++20 %t/use-header.cc -isystem %t -fsyntax-only 
-verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -isystem %t -emit-module-interface -o 
%t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/use-module.cc -isystem %t 
-fmodule-file=a=%t/a.pcm -fsyntax-only -verify
+
+//--- sys.h
+#ifndef SYS_H
+#define SYS_H
+
+#pragma GCC system_header
+
+template 
+struct [[deprecated]] iterator {};
+
+_Pragma("GCC diagnostic push")
+_Pragma("GCC diagnostic ignored \"-Wdeprecated\"") 
+_Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
+
+template 
+struct reverse_iterator 
+: public iterator {};
+
+_Pragma("GCC diagnostic pop")
+
+template 
+class C {
+public:
+void i() {
+reverse_iterator i;
+}
+};
+
+#endif
+
+//--- use-header.cc
+// expected-no-diagnostics
+// However, we see unexpected warnings
+#include 
+
+void use() {
+C().i();
+}
+
+//--- a.cppm
+module;
+#include 
+export module a;
+export using ::iterator;
+export using ::C;
+
+//--- use-module.cc
+// expected-no-diagnostics
+import a;
+
+void use() {
+C().i();
+}



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

[clang] 6b961e2 - Revert "[C++20] [Modules] Don't skip pragma diagnostic mappings"

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-04-30T14:28:11+08:00
New Revision: 6b961e2abfffd8b5a508b5958849b13b0feafa50

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

LOG: Revert "[C++20] [Modules] Don't skip pragma diagnostic mappings"
and "[NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to
generate module file for C++20 modules instead of PCHGenerator"

This reverts commit fb21343473e33e9a886b42d2fe95d1cec1cd0030.
and commit 18268ac0f48d93c2bcddb69732761971669c09ab.

It looks like there are some problems about linking the compiler

Added: 


Modified: 
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Serialization/GeneratePCH.cpp
clang/test/Modules/pr67893.cppm
clang/test/Modules/search-partitions.cpp

Removed: 
clang/test/Modules/pr75057.cppm



diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index 4e433deaaf2dbc..6c45b7348b8552 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -928,30 +928,17 @@ class PCHGenerator : public SemaConsumer {
   bool hasEmittedPCH() const { return Buffer->IsComplete; }
 };
 
-class CXX20ModulesGenerator : public PCHGenerator {
+class ReducedBMIGenerator : public PCHGenerator {
 protected:
   virtual Module *getEmittingModule(ASTContext &Ctx) override;
 
-  CXX20ModulesGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
-StringRef OutputFile, bool GeneratingReducedBMI);
-
 public:
-  CXX20ModulesGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
-StringRef OutputFile)
-  : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
-  /*GeneratingReducedBMI=*/false) {}
+  ReducedBMIGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
+  StringRef OutputFile);
 
   void HandleTranslationUnit(ASTContext &Ctx) override;
 };
 
-class ReducedBMIGenerator : public CXX20ModulesGenerator {
-public:
-  ReducedBMIGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
-  StringRef OutputFile)
-  : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
-  /*GeneratingReducedBMI=*/true) {}
-};
-
 /// If we can elide the definition of \param D in reduced BMI.
 ///
 /// Generally, we can elide the definition of a declaration if it won't affect

diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index 454653a31534cd..04eb1041326713 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -272,10 +272,14 @@ bool GenerateModuleInterfaceAction::BeginSourceFileAction(
 std::unique_ptr
 GenerateModuleInterfaceAction::CreateASTConsumer(CompilerInstance &CI,
  StringRef InFile) {
-  std::vector> Consumers;
-  Consumers.push_back(std::make_unique(
-  CI.getPreprocessor(), CI.getModuleCache(),
-  CI.getFrontendOpts().OutputFile));
+  CI.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true;
+  CI.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
+  CI.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings = true;
+
+  std::vector> Consumers =
+  CreateMultiplexConsumer(CI, InFile);
+  if (Consumers.empty())
+return nullptr;
 
   if (CI.getFrontendOpts().GenReducedBMI &&
   !CI.getFrontendOpts().ModuleOutputPath.empty()) {

diff  --git a/clang/lib/Serialization/GeneratePCH.cpp 
b/clang/lib/Serialization/GeneratePCH.cpp
index 53dda5f9a38580..bed74399098d7f 100644
--- a/clang/lib/Serialization/GeneratePCH.cpp
+++ b/clang/lib/Serialization/GeneratePCH.cpp
@@ -88,32 +88,36 @@ ASTDeserializationListener 
*PCHGenerator::GetASTDeserializationListener() {
   return &Writer;
 }
 
-CXX20ModulesGenerator::CXX20ModulesGenerator(Preprocessor &PP,
- InMemoryModuleCache &ModuleCache,
- StringRef OutputFile,
- bool GeneratingReducedBMI)
+ReducedBMIGenerator::ReducedBMIGenerator(Preprocessor &PP,
+ InMemoryModuleCache &ModuleCache,
+ StringRef OutputFile)
 : PCHGenerator(
   PP, ModuleCache, OutputFile, llvm::StringRef(),
   std::make_shared(),
   /*Extensions=*/ArrayRef>(),
   /*AllowASTWithErrors*/ false, /*IncludeTimestamps=*/false,
   /*BuildingImplicitModule=*/false, /*ShouldCacheASTInMemory=*/false,
-  GeneratingReducedBMI) {}
+  /*GeneratingReducedBMI=*/true) {}
 
-Module *CXX20ModulesGener

[clang] ec527b2 - [C++20] [Modules] Don't skip pragma diagnostic mappings

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-04-30T14:43:57+08:00
New Revision: ec527b21bb4196355184aa95ef31aa561b8e8b7b

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

LOG: [C++20] [Modules] Don't skip pragma diagnostic mappings

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

Previously, I thought the diagnostic mappings is not meaningful with
modules incorrectly. And this problem get revealed by another change
recently. So this patch tried to rever the previous "optimization"
partially.

Added: 
clang/test/Modules/pr75057.cppm

Modified: 
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Serialization/GeneratePCH.cpp

Removed: 




diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index 04eb1041326713..480dfa8c975933 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -274,7 +274,6 @@ 
GenerateModuleInterfaceAction::CreateASTConsumer(CompilerInstance &CI,
  StringRef InFile) {
   CI.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true;
   CI.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
-  CI.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings = true;
 
   std::vector> Consumers =
   CreateMultiplexConsumer(CI, InFile);

diff  --git a/clang/lib/Serialization/GeneratePCH.cpp 
b/clang/lib/Serialization/GeneratePCH.cpp
index bed74399098d7f..a2ddbe4624aae4 100644
--- a/clang/lib/Serialization/GeneratePCH.cpp
+++ b/clang/lib/Serialization/GeneratePCH.cpp
@@ -117,7 +117,6 @@ void ReducedBMIGenerator::HandleTranslationUnit(ASTContext 
&Ctx) {
   getPreprocessor().getHeaderSearchInfo().getHeaderSearchOpts();
   HSOpts.ModulesSkipDiagnosticOptions = true;
   HSOpts.ModulesSkipHeaderSearchPaths = true;
-  HSOpts.ModulesSkipPragmaDiagnosticMappings = true;
 
   PCHGenerator::HandleTranslationUnit(Ctx);
 

diff  --git a/clang/test/Modules/pr75057.cppm b/clang/test/Modules/pr75057.cppm
new file mode 100644
index 00..96781b3ccacc0b
--- /dev/null
+++ b/clang/test/Modules/pr75057.cppm
@@ -0,0 +1,66 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// Treat the behavior of using headers as baseline.
+// RUN: %clang_cc1 -std=c++20 %t/use-header.cc -isystem %t -fsyntax-only 
-verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -isystem %t -emit-module-interface -o 
%t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/use-module.cc -isystem %t 
-fmodule-file=a=%t/a.pcm -fsyntax-only -verify
+
+// Test again with reduced BMI.
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -isystem %t 
-emit-reduced-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/use-module.cc -isystem %t 
-fmodule-file=a=%t/a.pcm -fsyntax-only -verify
+
+//--- sys.h
+#ifndef SYS_H
+#define SYS_H
+
+#pragma GCC system_header
+
+template 
+struct [[deprecated]] iterator {};
+
+_Pragma("GCC diagnostic push")
+_Pragma("GCC diagnostic ignored \"-Wdeprecated\"") 
+_Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
+
+template 
+struct reverse_iterator 
+: public iterator {};
+
+_Pragma("GCC diagnostic pop")
+
+template 
+class C {
+public:
+void i() {
+reverse_iterator i;
+}
+};
+
+#endif
+
+//--- use-header.cc
+// expected-no-diagnostics
+// However, we see unexpected warnings
+#include 
+
+void use() {
+C().i();
+}
+
+//--- a.cppm
+module;
+#include 
+export module a;
+export using ::iterator;
+export using ::C;
+
+//--- use-module.cc
+// expected-no-diagnostics
+import a;
+
+void use() {
+C().i();
+}



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

[clang] [clang-tools-extra] [libcxx] [clang] Enable sized deallocation by default in C++14 onwards (PR #83774)

[Chuanqi Xu via cfe-commits]

@@ -0,0 +1,44 @@
+//===--- SizedDellocation.h - Sized Deallocation *- 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
+//
+//===--===//
+///
+/// \file
+/// Defines a function that returns the minimum OS versions supporting
+/// C++14's sized deallocation functions.
+///
+//===--===//
+
+#ifndef LLVM_CLANG_BASIC_SIZEDDEALLOCATION_H
+#define LLVM_CLANG_BASIC_SIZEDDEALLOCATION_H
+
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/VersionTuple.h"
+#include "llvm/TargetParser/Triple.h"
+
+namespace clang {
+inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) {

ChuanqiXu9 wrote:

If I read correctly, this is only used in `Darwin.cpp`, so it would be better 
to inline the function there.

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

[clang] [clang-tools-extra] [libcxx] [clang] Enable sized deallocation by default in C++14 onwards (PR #83774)

[Chuanqi Xu via cfe-commits]

@@ -72,6 +72,10 @@ sections with improvements to Clang's support for those 
languages.
 C++ Language Changes
 
 
+C++14 Feature Support
+^
+- Sized deallocation is enabled by default in C++14 onwards.

ChuanqiXu9 wrote:

It may be good to mention the flag enabled so that people met regression can 
disable it locally to workaround it.

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

[clang] [llvm] [coroutine] Implement llvm.coro.await.suspend intrinsic (PR #79712)

[Chuanqi Xu via cfe-commits]

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

[clang] [llvm] [coroutine] Implement llvm.coro.await.suspend intrinsic (PR #79712)

[Chuanqi Xu via cfe-commits]

@@ -338,6 +414,71 @@ static QualType getCoroutineSuspendExprReturnType(const 
ASTContext &Ctx,
 }
 #endif
 
+llvm::Function *
+CodeGenFunction::generateAwaitSuspendWrapper(Twine const &CoroName,
+ Twine const &SuspendPointName,
+ CoroutineSuspendExpr const &S) {
+  std::string FuncName = "__await_suspend_wrapper_";
+  FuncName += CoroName.str();
+  FuncName += '_';
+  FuncName += SuspendPointName.str();
+
+  ASTContext &C = getContext();
+
+  FunctionArgList args;
+
+  ImplicitParamDecl AwaiterDecl(C, C.VoidPtrTy, ImplicitParamKind::Other);
+  ImplicitParamDecl FrameDecl(C, C.VoidPtrTy, ImplicitParamKind::Other);
+  QualType ReturnTy = S.getSuspendExpr()->getType();
+
+  args.push_back(&AwaiterDecl);
+  args.push_back(&FrameDecl);
+
+  const CGFunctionInfo &FI =
+  CGM.getTypes().arrangeBuiltinFunctionDeclaration(ReturnTy, args);
+
+  llvm::FunctionType *LTy = CGM.getTypes().GetFunctionType(FI);
+
+  llvm::Function *Fn = llvm::Function::Create(
+  LTy, llvm::GlobalValue::PrivateLinkage, FuncName, &CGM.getModule());
+
+  Fn->addParamAttr(0, llvm::Attribute::AttrKind::NonNull);
+  Fn->addParamAttr(0, llvm::Attribute::AttrKind::NoUndef);
+
+  Fn->addParamAttr(1, llvm::Attribute::AttrKind::NoUndef);
+
+  Fn->setMustProgress();
+  Fn->addFnAttr(llvm::Attribute::AttrKind::AlwaysInline);
+
+  StartFunction(GlobalDecl(), ReturnTy, Fn, FI, args);
+
+  llvm::Value *AwaiterPtr = 
Builder.CreateLoad(GetAddrOfLocalVar(&AwaiterDecl));
+  auto AwaiterLValue =
+  MakeNaturalAlignAddrLValue(AwaiterPtr, AwaiterDecl.getType());
+
+  CurAwaitSuspendWrapper.FramePtr =
+  Builder.CreateLoad(GetAddrOfLocalVar(&FrameDecl));
+
+  // FIXME: mark as aliasing with frame?
+  // FIXME: TBAA?
+  // FIXME: emit in a better way (maybe egenerate AST in SemaCoroutine)?

ChuanqiXu9 wrote:

Generally we like comments in a more formal style. Please rewrite the comments 
to make it more expressive.

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

[clang] [llvm] [coroutine] Implement llvm.coro.await.suspend intrinsic (PR #79712)

[Chuanqi Xu via cfe-commits]

@@ -167,6 +167,53 @@ class CoroCloner {
 
 } // end anonymous namespace
 
+// FIXME:
+// Lower the intrinisc in CoroEarly phase if coroutine frame doesn't escape
+// and it is known that other transformations, for example, sanitizers
+// won't lead to incorrect code.
+static void lowerAwaitSuspend(IRBuilder<> &Builder, CoroAwaitSuspendInst *CB) {
+  auto Wrapper = CB->getWrapperFunction();
+  auto Awaiter = CB->getAwaiter();
+  auto FramePtr = CB->getFrame();
+
+  Builder.SetInsertPoint(CB);
+
+  CallBase *NewCall = nullptr;
+  if (auto Invoke = dyn_cast(CB)) {
+auto WrapperInvoke =
+Builder.CreateInvoke(Wrapper, Invoke->getNormalDest(),
+ Invoke->getUnwindDest(), {Awaiter, FramePtr});
+
+WrapperInvoke->setCallingConv(Invoke->getCallingConv());
+std::copy(Invoke->bundle_op_info_begin(), Invoke->bundle_op_info_end(),
+  WrapperInvoke->bundle_op_info_begin());
+AttributeList NewAttributes =
+Invoke->getAttributes().removeParamAttributes(Invoke->getContext(), 2);

ChuanqiXu9 wrote:

What is the attribute to be removed? Let's add a comment here to make it clear.

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

[clang] [llvm] [coroutine] Implement llvm.coro.await.suspend intrinsic (PR #79712)

[Chuanqi Xu via cfe-commits]

https://github.com/ChuanqiXu9 commented:

Thanks. This looks good to me except few nit comments.

Have you tested on a real world workloads?

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

[clang] [clang][NFC] Format clang/lib/Sema/Sema.cpp (PR #83974)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

I remember we have policies that we don't like patches which purely formats 
codes. It makes backporting and cherry-picking harder. But I can't find the 
wording now.

CC: @AaronBallman @Endilll 

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

[clang] [clang][NFC] Format clang/lib/Sema/Sema.cpp (PR #83974)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

Oh, I found it here: https://llvm.org/docs/CodingStandards.html#introduction

> Our long term goal is for the entire codebase to follow the convention, but 
> we explicitly do not want patches that do large-scale reformatting of 
> existing code.

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

[clang] [clang][NFC] Format clang/lib/Sema/Sema.cpp (PR #83974)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> Thank you for references both.
> 
> Actually, I'd like to have PR like #83961. Is it acceptable to merge this 
> kind of PR in that case then?

I don't feel the patches are related. I think you can only format the changed 
lines.

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

[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> * I do not want to block progress, so let's move forward with this patch for 
> now.

Yeah. Great to see we have some progress finally. I think this is really 
important since I see more and more peope complaninig the performance for 
modules. I feel this series patch is key to to improve the user's impression 
that named modules are just purely wrappers for PCH.

> * It seems to me (as we found with GMF decl elision) that the process is 
> quite a bit more complex than simply omitting a decl.  We need to elide other 
> decls that are then unused (e.g. decls local to an elided function body) and 
> also avoid emitting types that are now no longer existent.

After looking into the codes more, I changed my mind for this. I feel it is the 
most efficient and the most natural way to omit declarations in ASTWriter.

The idea is, previously, we'll start to write declarations in the current TU 
from the first declarations. And in the C++20 named modules world, we would 
start to write declarations from the first declarations in the global module 
fragment.  And we can implement GMF decl elision naturally by writing 
declarations from the first declaration in the module purview and only write 
the referenced decl from the GMF during the writing process. This is super 
natural and efficient.

> 
> The process seems to me to be either one of:
> 
> * rewriting the AST (which is why my patch set picked the use of the plugin 
> API since that is the purpose there).
> * walking the AST and marking entities as used / not used / elided.

Now I doubt both of the method to be not efficiency and it adds additional 
burdens to the developers and the users.

> 
> It still feels to me to be better to have clear separation of this work from 
> the work of streaming - but if we can make clear layers within the streaming, 
> then maybe the maintenance will not be too hard.

I think the maintainance may not be too hard in that way. ASTWriter is not such 
a devil :) Probably we need some refactoration in the serialization to make 
codes more clear. But the point is that we must get the ASTWriter/ASTReader 
involved. It may not be a good idea to leave the ASTWriter/ASTReader as is and 
add another layer... 

For example, it should be better to not deserialize the declaration from the 
very beginning instead of deserializing the declaration and judge that it is 
not wanted/visible. And to achieve this, we must  touch the serialization layer 
deeply.



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

[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)

[Chuanqi Xu via cfe-commits]

@@ -830,6 +843,19 @@ class PCHGenerator : public SemaConsumer {
   bool hasEmittedPCH() const { return Buffer->IsComplete; }
 };
 
+class ReducedBMIGenerator : public PCHGenerator {
+public:
+  ReducedBMIGenerator(const Preprocessor &PP, InMemoryModuleCache &ModuleCache,
+  StringRef OutputFile, std::shared_ptr Buffer,
+  bool IncludeTimestamps);
+
+  void HandleTranslationUnit(ASTContext &Ctx) override;
+};
+
+/// If the definition may impact the ABI. If yes, we're allowed to eliminate
+/// the definition of D in reduced BMI.
+bool MayDefAffectABI(const Decl *D);

ChuanqiXu9 wrote:

Nice catch up.

I changed the name to `CanElideDeclDef` and the comments to:

```
/// If we can elide the definition of \param D in reduced BMI.
///
/// Generally, we can elide the definition of a declaration if it won't affect 
the
/// ABI. e.g., the non-inline function bodies.
```

Hope this to be clear.

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

[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> Do you expect to make any changes to type streaming?

I don't expect to do that explicitly. The number of types deserialized can be 
decreased naturally after we avoid emitting declarations during the writing.

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

[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

I am going to land this in the week later if no objections come in. I think it 
is necessary to land the series of patches (to reduce the contents of BMI) for 
clang19. And of course, the functionality will be opt in for one~two releases 
for experimental.

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

[clang] d3df2a8 - [C++20] [Modules] Handle transitive import in the module properly

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-03-06T15:46:55+08:00
New Revision: d3df2a834cf6febb44c699d109b9e7f622194837

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

LOG: [C++20] [Modules] Handle transitive import in the module properly

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

Per [module.import]p7:

> Additionally, when a module-import-declaration in a module unit of
> some module M imports another module unit U of M, it also imports all
> translation units imported by non-exported module-import-declarations
> in the module unit purview of U.

However, we only tried to implement it during the implicit import of
primary module interface for module implementation unit.

Also we didn't implement the last sentence from [module.import]p7
completely:

> These rules can in turn lead to the importation of yet more
> translation units.

This patch tries to care the both issues.

Added: 
clang/test/Modules/transitive-import.cppm

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Module.h
clang/lib/Basic/Module.cpp
clang/lib/Sema/SemaModule.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5e0352a7eaf6cd..b074055a4eaf69 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -86,6 +86,11 @@ C++20 Feature Support
 - Implemented the `__is_layout_compatible` intrinsic to support
   `P0466R5: Layout-compatibility and Pointer-interconvertibility Traits 
`_.
 
+- Clang now implements [module.import]p7 fully. Clang now will import module
+  units transitively for the module units coming from the same module of the
+  current module units.
+  Fixes `#84002 `_.
+
 C++23 Feature Support
 ^
 

diff  --git a/clang/include/clang/Basic/Module.h 
b/clang/include/clang/Basic/Module.h
index 30ec9c99315092..9f62c058ca0da0 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -598,6 +598,11 @@ class alignas(8) Module {
Kind == ModulePartitionImplementation;
   }
 
+  /// Is this a module partition implementation unit.
+  bool isModulePartitionImplementation() const {
+return Kind == ModulePartitionImplementation;
+  }
+
   /// Is this a module implementation.
   bool isModuleImplementation() const {
 return Kind == ModuleImplementationUnit;
@@ -853,12 +858,6 @@ class VisibleModuleSet {
   VisibleCallback Vis = [](Module *) {},
   ConflictCallback Cb = [](ArrayRef, Module *,
StringRef) {});
-
-  /// Make transitive imports visible for [module.import]/7.
-  void makeTransitiveImportsVisible(
-  Module *M, SourceLocation Loc, VisibleCallback Vis = [](Module *) {},
-  ConflictCallback Cb = [](ArrayRef, Module *, StringRef) {});
-
 private:
   /// Import locations for each visible module. Indexed by the module's
   /// VisibilityID.

diff  --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 1c5043a618fff3..9f597dcf8b0f5a 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -722,14 +722,6 @@ void VisibleModuleSet::setVisible(Module *M, 
SourceLocation Loc,
   VisitModule({M, nullptr});
 }
 
-void VisibleModuleSet::makeTransitiveImportsVisible(Module *M,
-SourceLocation Loc,
-VisibleCallback Vis,
-ConflictCallback Cb) {
-  for (auto *I : M->Imports)
-setVisible(I, Loc, Vis, Cb);
-}
-
 ASTSourceDescriptor::ASTSourceDescriptor(Module &M)
 : Signature(M.Signature), ClangModule(&M) {
   if (M.Directory)

diff  --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index ed7f626971f345..f08c1cb3a13ef5 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -73,6 +73,90 @@ static std::string stringFromPath(ModuleIdPath Path) {
   return Name;
 }
 
+/// Helper function for makeTransitiveImportsVisible to decide whether
+/// the \param Imported module unit is in the same module with the \param
+/// CurrentModule.
+/// \param FoundPrimaryModuleInterface is a helper parameter to record the
+/// primary module interface unit corresponding to the module \param
+/// CurrentModule. Since currently it is expensive to decide whether two module
+/// units come from the same module by comparing the module name.
+static bool
+isImportingModuleUnitFromSameModule(Module *Imported, Module *CurrentModule,
+Module *&FoundPrimaryModuleInterface) {
+  if (!Imported->isNamedModule())
+return false;
+
+  // The a partition 

[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> Can we add a release note and documentation for this? Thanks!

The current patch is transparent to users and it is only part of the series 
patches. I'd like to document that after I made the series of patches.

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

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Chuanqi Xu via cfe-commits]

@@ -6898,10 +6898,18 @@ class Sema final {
BinaryOperatorKind Operator);
 
    ActOnCXXThis -  Parse 'this' pointer.
-  ExprResult ActOnCXXThis(SourceLocation loc);
+  ///
+  /// \param SkipLambdaCaptureCheck Whether to skip the 'this' check for a
+  /// lambda because 'this' is the lambda's 'this'-pointer.
+  ExprResult ActOnCXXThis(SourceLocation loc,
+  bool SkipLambdaCaptureCheck = false);

ChuanqiXu9 wrote:

Can we provide more rationale for skipping lambda's capture check here? For 
example, in what cases and for what reasons, we want to skip the check. I am 
still confused for that.

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

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Chuanqi Xu via cfe-commits]

@@ -6898,10 +6898,18 @@ class Sema final {
BinaryOperatorKind Operator);
 
    ActOnCXXThis -  Parse 'this' pointer.
-  ExprResult ActOnCXXThis(SourceLocation loc);
+  ///
+  /// \param SkipLambdaCaptureCheck Whether to skip the 'this' check for a
+  /// lambda because 'this' is the lambda's 'this'-pointer.
+  ExprResult ActOnCXXThis(SourceLocation loc,
+  bool SkipLambdaCaptureCheck = false);

ChuanqiXu9 wrote:

Excellent. Good. I suggest you to update this to the inline comments.

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

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Chuanqi Xu via cfe-commits]

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

LGTM with the adding the comments.

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

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Chuanqi Xu via cfe-commits]

@@ -6898,10 +6898,18 @@ class Sema final {
BinaryOperatorKind Operator);
 
    ActOnCXXThis -  Parse 'this' pointer.
-  ExprResult ActOnCXXThis(SourceLocation loc);
+  ///
+  /// \param SkipLambdaCaptureCheck Whether to skip the 'this' check for a
+  /// lambda because 'this' is the lambda's 'this'-pointer.
+  ExprResult ActOnCXXThis(SourceLocation loc,
+  bool SkipLambdaCaptureCheck = false);

ChuanqiXu9 wrote:

If there is any problem with the promise_type with a constructor, let's make it 
in other patches.

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

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

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


I suddenly found that the clang scan deps may use all concurrent threads to 
scan the files. It makes sense in the batch mode. But in P1689 per file mode, 
it simply wastes times and resources.

This patch itself should be a NFC patch. It simply moves codes.

>From 8f3060da8059dc300152a1a1bc297cffcba51a61 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Thu, 7 Mar 2024 15:19:28 +0800
Subject: [PATCH] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread
 pool in P1689 per file mode

I suddenly found that the clang scan deps may use all concurrent threads
to scan the files. It makes sense in the batch mode. But in P1689
per file mode, it simply wastes times.
---
 clang/tools/clang-scan-deps/ClangScanDeps.cpp | 204 +-
 1 file changed, 108 insertions(+), 96 deletions(-)

diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index d042fecc3dbe63..843816a8ed6515 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -744,6 +744,9 @@ getCompilationDataBase(int argc, char **argv, std::string 
&ErrorMessage) {
 return nullptr;
   }
 
+  // Only 1 threads is required if P1689 per file mode.
+  NumThreads = 1;
+
   // There might be multiple jobs for a compilation. Extract the specified
   // output filename from the last job.
   auto LastCmd = C->getJobs().end();
@@ -867,13 +870,6 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());
 
-  DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
-EagerLoadModules);
-  llvm::DefaultThreadPool Pool(llvm::hardware_concurrency(NumThreads));
-  std::vector> WorkerTools;
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I)
-WorkerTools.push_back(std::make_unique(Service));
-
   std::vector Inputs =
   AdjustingCompilations->getAllCompileCommands();
 
@@ -893,102 +889,118 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   if (Format == ScanningOutputFormat::Full)
 FD.emplace(ModuleName.empty() ? Inputs.size() : 0);
 
-  if (Verbose) {
-llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
- << " files using " << Pool.getMaxConcurrency() << " 
workers\n";
-  }
-
-  llvm::Timer T;
-  T.startTimer();
-
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I) {
-Pool.async([&, I]() {
-  llvm::DenseSet AlreadySeenModules;
-  while (auto MaybeInputIndex = GetNextInputIndex()) {
-size_t LocalIndex = *MaybeInputIndex;
-const tooling::CompileCommand *Input = &Inputs[LocalIndex];
-std::string Filename = std::move(Input->Filename);
-std::string CWD = std::move(Input->Directory);
-
-std::optional MaybeModuleName;
-if (!ModuleName.empty())
-  MaybeModuleName = ModuleName;
-
-std::string OutputDir(ModuleFilesDir);
-if (OutputDir.empty())
-  OutputDir = getModuleCachePath(Input->CommandLine);
-auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
-  return ::lookupModuleOutput(MID, MOK, OutputDir);
-};
-
-// Run the tool on it.
-if (Format == ScanningOutputFormat::Make) {
-  auto MaybeFile =
-  WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
-  if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
- Errs))
-HadErrors = true;
-} else if (Format == ScanningOutputFormat::P1689) {
-  // It is useful to generate the make-format dependency output during
-  // the scanning for P1689. Otherwise the users need to scan again for
-  // it. We will generate the make-format dependency output if we find
-  // `-MF` in the command lines.
-  std::string MakeformatOutputPath;
-  std::string MakeformatOutput;
-
-  auto MaybeRule = WorkerTools[I]->getP1689ModuleDependencyFile(
-  *Input, CWD, MakeformatOutput, MakeformatOutputPath);
-
-  if (handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs))
-HadErrors = true;
+  std::vector> WorkerTools;
 
-  if (!MakeformatOutputPath.empty() && !MakeformatOutput.empty() &&
-  !HadErrors) {
-static std::mutex Lock;
-// With compilation database, we may open different files
-// concurrently or we may write the same file concurrently. So we
-// use a map here to allow multiple compile commands to write to 
the
-// same file. Also we need a lock here to avoid data race.
-static llvm::StringMap OSs;
-std::unique_lock LockGuard(Lock);
-

[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)

[Chuanqi Xu via cfe-commits]

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

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

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

>From dd9711be2368a299b408d0ff06ec9c1c0540083b Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Thu, 7 Mar 2024 15:19:28 +0800
Subject: [PATCH 1/2] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use
 thread pool in P1689 per file mode

I suddenly found that the clang scan deps may use all concurrent threads
to scan the files. It makes sense in the batch mode. But in P1689
per file mode, it simply wastes times.
---
 clang/tools/clang-scan-deps/ClangScanDeps.cpp | 204 +-
 1 file changed, 108 insertions(+), 96 deletions(-)

diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index d042fecc3dbe63..843816a8ed6515 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -744,6 +744,9 @@ getCompilationDataBase(int argc, char **argv, std::string 
&ErrorMessage) {
 return nullptr;
   }
 
+  // Only 1 threads is required if P1689 per file mode.
+  NumThreads = 1;
+
   // There might be multiple jobs for a compilation. Extract the specified
   // output filename from the last job.
   auto LastCmd = C->getJobs().end();
@@ -867,13 +870,6 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());
 
-  DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
-EagerLoadModules);
-  llvm::DefaultThreadPool Pool(llvm::hardware_concurrency(NumThreads));
-  std::vector> WorkerTools;
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I)
-WorkerTools.push_back(std::make_unique(Service));
-
   std::vector Inputs =
   AdjustingCompilations->getAllCompileCommands();
 
@@ -893,102 +889,118 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   if (Format == ScanningOutputFormat::Full)
 FD.emplace(ModuleName.empty() ? Inputs.size() : 0);
 
-  if (Verbose) {
-llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
- << " files using " << Pool.getMaxConcurrency() << " 
workers\n";
-  }
-
-  llvm::Timer T;
-  T.startTimer();
-
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I) {
-Pool.async([&, I]() {
-  llvm::DenseSet AlreadySeenModules;
-  while (auto MaybeInputIndex = GetNextInputIndex()) {
-size_t LocalIndex = *MaybeInputIndex;
-const tooling::CompileCommand *Input = &Inputs[LocalIndex];
-std::string Filename = std::move(Input->Filename);
-std::string CWD = std::move(Input->Directory);
-
-std::optional MaybeModuleName;
-if (!ModuleName.empty())
-  MaybeModuleName = ModuleName;
-
-std::string OutputDir(ModuleFilesDir);
-if (OutputDir.empty())
-  OutputDir = getModuleCachePath(Input->CommandLine);
-auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
-  return ::lookupModuleOutput(MID, MOK, OutputDir);
-};
-
-// Run the tool on it.
-if (Format == ScanningOutputFormat::Make) {
-  auto MaybeFile =
-  WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
-  if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
- Errs))
-HadErrors = true;
-} else if (Format == ScanningOutputFormat::P1689) {
-  // It is useful to generate the make-format dependency output during
-  // the scanning for P1689. Otherwise the users need to scan again for
-  // it. We will generate the make-format dependency output if we find
-  // `-MF` in the command lines.
-  std::string MakeformatOutputPath;
-  std::string MakeformatOutput;
-
-  auto MaybeRule = WorkerTools[I]->getP1689ModuleDependencyFile(
-  *Input, CWD, MakeformatOutput, MakeformatOutputPath);
-
-  if (handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs))
-HadErrors = true;
+  std::vector> WorkerTools;
 
-  if (!MakeformatOutputPath.empty() && !MakeformatOutput.empty() &&
-  !HadErrors) {
-static std::mutex Lock;
-// With compilation database, we may open different files
-// concurrently or we may write the same file concurrently. So we
-// use a map here to allow multiple compile commands to write to 
the
-// same file. Also we need a lock here to avoid data race.
-static llvm::StringMap OSs;
-std::unique_lock LockGuard(Lock);
-
-auto OSIter = OSs.find(MakeformatOutputPath);
-if (OSIter == OSs.end()) {
-  std::error_code EC;
-  OSIter = OSs.try_emplace(MakeformatOutputPath,
-   MakeformatOutputPath

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

@@ -744,6 +744,9 @@ getCompilationDataBase(int argc, char **argv, std::string 
&ErrorMessage) {
 return nullptr;
   }
 
+  // Only 1 threads is required if P1689 per file mode.
+  NumThreads = 1;

ChuanqiXu9 wrote:

Makes sense. Done.

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

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

@@ -893,102 +889,118 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   if (Format == ScanningOutputFormat::Full)
 FD.emplace(ModuleName.empty() ? Inputs.size() : 0);
 
-  if (Verbose) {
-llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
- << " files using " << Pool.getMaxConcurrency() << " 
workers\n";
-  }
-
-  llvm::Timer T;
-  T.startTimer();
-
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I) {
-Pool.async([&, I]() {
-  llvm::DenseSet AlreadySeenModules;
-  while (auto MaybeInputIndex = GetNextInputIndex()) {
-size_t LocalIndex = *MaybeInputIndex;
-const tooling::CompileCommand *Input = &Inputs[LocalIndex];
-std::string Filename = std::move(Input->Filename);
-std::string CWD = std::move(Input->Directory);
-
-std::optional MaybeModuleName;
-if (!ModuleName.empty())
-  MaybeModuleName = ModuleName;
-
-std::string OutputDir(ModuleFilesDir);
-if (OutputDir.empty())
-  OutputDir = getModuleCachePath(Input->CommandLine);
-auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
-  return ::lookupModuleOutput(MID, MOK, OutputDir);
-};
-
-// Run the tool on it.
-if (Format == ScanningOutputFormat::Make) {
-  auto MaybeFile =
-  WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
-  if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
- Errs))
-HadErrors = true;
-} else if (Format == ScanningOutputFormat::P1689) {
-  // It is useful to generate the make-format dependency output during
-  // the scanning for P1689. Otherwise the users need to scan again for
-  // it. We will generate the make-format dependency output if we find
-  // `-MF` in the command lines.
-  std::string MakeformatOutputPath;
-  std::string MakeformatOutput;
-
-  auto MaybeRule = WorkerTools[I]->getP1689ModuleDependencyFile(
-  *Input, CWD, MakeformatOutput, MakeformatOutputPath);
-
-  if (handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs))
-HadErrors = true;
+  std::vector> WorkerTools;
 
-  if (!MakeformatOutputPath.empty() && !MakeformatOutput.empty() &&
-  !HadErrors) {
-static std::mutex Lock;
-// With compilation database, we may open different files
-// concurrently or we may write the same file concurrently. So we
-// use a map here to allow multiple compile commands to write to 
the
-// same file. Also we need a lock here to avoid data race.
-static llvm::StringMap OSs;
-std::unique_lock LockGuard(Lock);
-
-auto OSIter = OSs.find(MakeformatOutputPath);
-if (OSIter == OSs.end()) {
-  std::error_code EC;
-  OSIter = OSs.try_emplace(MakeformatOutputPath,
-   MakeformatOutputPath, EC)
-   .first;
-  if (EC)
-llvm::errs()
-<< "Failed to open P1689 make format output file \""
-<< MakeformatOutputPath << "\" for " << EC.message()
-<< "\n";
-}
+  auto ScanningTask = [&](unsigned I) {

ChuanqiXu9 wrote:

Done. It looks indeed better.

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

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

@@ -893,102 +889,118 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   if (Format == ScanningOutputFormat::Full)
 FD.emplace(ModuleName.empty() ? Inputs.size() : 0);
 
-  if (Verbose) {
-llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
- << " files using " << Pool.getMaxConcurrency() << " 
workers\n";
-  }
-
-  llvm::Timer T;
-  T.startTimer();
-
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I) {
-Pool.async([&, I]() {
-  llvm::DenseSet AlreadySeenModules;
-  while (auto MaybeInputIndex = GetNextInputIndex()) {
-size_t LocalIndex = *MaybeInputIndex;
-const tooling::CompileCommand *Input = &Inputs[LocalIndex];
-std::string Filename = std::move(Input->Filename);
-std::string CWD = std::move(Input->Directory);
-
-std::optional MaybeModuleName;
-if (!ModuleName.empty())
-  MaybeModuleName = ModuleName;
-
-std::string OutputDir(ModuleFilesDir);
-if (OutputDir.empty())
-  OutputDir = getModuleCachePath(Input->CommandLine);
-auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
-  return ::lookupModuleOutput(MID, MOK, OutputDir);
-};
-
-// Run the tool on it.
-if (Format == ScanningOutputFormat::Make) {
-  auto MaybeFile =
-  WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
-  if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
- Errs))
-HadErrors = true;
-} else if (Format == ScanningOutputFormat::P1689) {
-  // It is useful to generate the make-format dependency output during
-  // the scanning for P1689. Otherwise the users need to scan again for
-  // it. We will generate the make-format dependency output if we find
-  // `-MF` in the command lines.
-  std::string MakeformatOutputPath;
-  std::string MakeformatOutput;
-
-  auto MaybeRule = WorkerTools[I]->getP1689ModuleDependencyFile(
-  *Input, CWD, MakeformatOutput, MakeformatOutputPath);
-
-  if (handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs))
-HadErrors = true;
+  std::vector> WorkerTools;

ChuanqiXu9 wrote:

Done by moving this into the `ThreadPool` only branch and get rid of 
`std::unique_ptr`. It looks like we can't remove the vector entirely. Since the 
parameter type of `ScanningTask` is `DependencyScanningTool &` instead of 
`const DependencyScanningTool &`, so we may face lifetime issues if we 
construct DependencyScanningTool in place.

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

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

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

>From dd9711be2368a299b408d0ff06ec9c1c0540083b Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Thu, 7 Mar 2024 15:19:28 +0800
Subject: [PATCH 1/3] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use
 thread pool in P1689 per file mode

I suddenly found that the clang scan deps may use all concurrent threads
to scan the files. It makes sense in the batch mode. But in P1689
per file mode, it simply wastes times.
---
 clang/tools/clang-scan-deps/ClangScanDeps.cpp | 204 +-
 1 file changed, 108 insertions(+), 96 deletions(-)

diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index d042fecc3dbe63..843816a8ed6515 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -744,6 +744,9 @@ getCompilationDataBase(int argc, char **argv, std::string 
&ErrorMessage) {
 return nullptr;
   }
 
+  // Only 1 threads is required if P1689 per file mode.
+  NumThreads = 1;
+
   // There might be multiple jobs for a compilation. Extract the specified
   // output filename from the last job.
   auto LastCmd = C->getJobs().end();
@@ -867,13 +870,6 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());
 
-  DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
-EagerLoadModules);
-  llvm::DefaultThreadPool Pool(llvm::hardware_concurrency(NumThreads));
-  std::vector> WorkerTools;
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I)
-WorkerTools.push_back(std::make_unique(Service));
-
   std::vector Inputs =
   AdjustingCompilations->getAllCompileCommands();
 
@@ -893,102 +889,118 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   if (Format == ScanningOutputFormat::Full)
 FD.emplace(ModuleName.empty() ? Inputs.size() : 0);
 
-  if (Verbose) {
-llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
- << " files using " << Pool.getMaxConcurrency() << " 
workers\n";
-  }
-
-  llvm::Timer T;
-  T.startTimer();
-
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I) {
-Pool.async([&, I]() {
-  llvm::DenseSet AlreadySeenModules;
-  while (auto MaybeInputIndex = GetNextInputIndex()) {
-size_t LocalIndex = *MaybeInputIndex;
-const tooling::CompileCommand *Input = &Inputs[LocalIndex];
-std::string Filename = std::move(Input->Filename);
-std::string CWD = std::move(Input->Directory);
-
-std::optional MaybeModuleName;
-if (!ModuleName.empty())
-  MaybeModuleName = ModuleName;
-
-std::string OutputDir(ModuleFilesDir);
-if (OutputDir.empty())
-  OutputDir = getModuleCachePath(Input->CommandLine);
-auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
-  return ::lookupModuleOutput(MID, MOK, OutputDir);
-};
-
-// Run the tool on it.
-if (Format == ScanningOutputFormat::Make) {
-  auto MaybeFile =
-  WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
-  if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
- Errs))
-HadErrors = true;
-} else if (Format == ScanningOutputFormat::P1689) {
-  // It is useful to generate the make-format dependency output during
-  // the scanning for P1689. Otherwise the users need to scan again for
-  // it. We will generate the make-format dependency output if we find
-  // `-MF` in the command lines.
-  std::string MakeformatOutputPath;
-  std::string MakeformatOutput;
-
-  auto MaybeRule = WorkerTools[I]->getP1689ModuleDependencyFile(
-  *Input, CWD, MakeformatOutput, MakeformatOutputPath);
-
-  if (handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs))
-HadErrors = true;
+  std::vector> WorkerTools;
 
-  if (!MakeformatOutputPath.empty() && !MakeformatOutput.empty() &&
-  !HadErrors) {
-static std::mutex Lock;
-// With compilation database, we may open different files
-// concurrently or we may write the same file concurrently. So we
-// use a map here to allow multiple compile commands to write to 
the
-// same file. Also we need a lock here to avoid data race.
-static llvm::StringMap OSs;
-std::unique_lock LockGuard(Lock);
-
-auto OSIter = OSs.find(MakeformatOutputPath);
-if (OSIter == OSs.end()) {
-  std::error_code EC;
-  OSIter = OSs.try_emplace(MakeformatOutputPath,
-   MakeformatOutputPath

[clang] [llvm] [coroutine] Implement llvm.coro.await.suspend intrinsic (PR #79712)

[Chuanqi Xu via cfe-commits]

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

LGTM then. Thanks.

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

[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

[Chuanqi Xu via cfe-commits]

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

LGTM.

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

[clang] [llvm] [coroutine] Implement llvm.coro.await.suspend intrinsic (PR #79712)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

I'll merge this since I find you may not have commit access.

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

[clang] [llvm] [coroutine] Implement llvm.coro.await.suspend intrinsic (PR #79712)

[Chuanqi Xu via cfe-commits]

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

[clang] 3f6bc1a - [C++20] [Moduls] Avoid computing odr hash for functions from comparing constraint expression

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-03-11T11:39:21+08:00
New Revision: 3f6bc1adf805681293c2ef0b93b708ff52244c00

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

LOG: [C++20] [Moduls] Avoid computing odr hash for functions from comparing 
constraint expression

Previously we disabled to compute ODR hash for declarations from the
global module fragment. However, we missed the case that the functions
lives in the concept requiments (see the attached the test files for
example). And the mismatch causes the potential crashment.

Due to we will set the function body as lazy after we deserialize it and
we will only take its body when needed. However, we don't allow to take
the body during deserializing. So it is actually potentially problematic
if we set the body as lazy first and computing the hash value of the
function, which requires to deserialize its body. So we will meet a
crash here.

This patch tries to solve the issue by not taking the body of the
function from GMF. Note that we can't skip comparing the constraint
expression from the GMF directly since it is an key part of the
function selecting and it may be the reason why we can't return 0
directly for `FunctionDecl::getODRHash()` from the GMF.

Added: 
clang/test/Modules/hashing-decls-in-exprs-from-gmf.cppm

Modified: 
clang/include/clang/AST/DeclBase.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterDecl.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 76810a86a78a46..47ed6d0d1db0df 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -673,6 +673,16 @@ class alignas(8) Decl {
   /// fragment. See [module.global.frag]p3,4 for details.
   bool isDiscardedInGlobalModuleFragment() const { return false; }
 
+  /// Check if we should skip checking ODRHash for declaration \param D.
+  ///
+  /// The existing ODRHash mechanism seems to be not stable enough and
+  /// the false positive ODR violation reports are annoying and we rarely see
+  /// true ODR violation reports. Also we learned that MSVC disabled ODR checks
+  /// for declarations in GMF. So we try to disable ODR checks in the GMF to
+  /// get better user experiences before we make the ODR violation checks 
stable
+  /// enough.
+  bool shouldSkipCheckingODR() 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/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 2002bf23c9595f..370d8037a4da17 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2456,13 +2456,6 @@ class BitsUnpacker {
   uint32_t Value;
   uint32_t CurrentBitsIndex = ~0;
 };
-
-inline bool shouldSkipCheckingODR(const Decl *D) {
-  return D->getOwningModule() &&
- D->getASTContext().getLangOpts().SkipODRCheckInGMF &&
- D->getOwningModule()->isExplicitGlobalModule();
-}
-
 } // namespace clang
 
 #endif // LLVM_CLANG_SERIALIZATION_ASTREADER_H

diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index d681791d3920c3..8626f04012f7d4 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4496,7 +4496,7 @@ unsigned FunctionDecl::getODRHash() {
   }
 
   class ODRHash Hash;
-  Hash.AddFunctionDecl(this);
+  Hash.AddFunctionDecl(this, /*SkipBody=*/shouldSkipCheckingODR());
   setHasODRHash(true);
   ODRHash = Hash.CalculateHash();
   return ODRHash;

diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index fcedb3cfd176a0..04bbc49ab2f319 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1102,6 +1102,11 @@ bool Decl::isInAnotherModuleUnit() const {
   return M != getASTContext().getCurrentNamedModule();
 }
 
+bool Decl::shouldSkipCheckingODR() const {
+  return getASTContext().getLangOpts().SkipODRCheckInGMF && getOwningModule() 
&&
+ getOwningModule()->isExplicitGlobalModule();
+}
+
 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/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 683a076e6bc399..ede9f6e93469b7 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9762,7 +9762,7 @@ void ASTReader::finishPendingActions() {
 !NonConstDefn->isLateTemplateParsed

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

This commit breaks our coroutines library async_simple 
`https://github.com/alibaba/async_simple` and here is a (relative) minimal 
reproducer: https://godbolt.org/z/sG5jzcGEz

The reproducer comes from an implementation for async_simple::Generator 
(https://github.com/alibaba/async_simple/blob/main/async_simple/coro/Generator.h),
 which is basically the same implementation from the paper of `std::generator` 
in c++23. So possibly this may make that crash too. 

I'll revert this patch to avoid the regression now. Sorry for not testing it 
sufficiently.

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

[clang] 0f501c3 - Revert "[C++20][Coroutines] Lambda-coroutine with operator new in promise_type (#84193)"

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-03-11T17:02:43+08:00
New Revision: 0f501c30b9601627c236f9abca8a3befba5dc161

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

LOG: Revert "[C++20][Coroutines] Lambda-coroutine with operator new in 
promise_type (#84193)"

This reverts commit 35d3b33ba5c9b90443ac985f2521b78f84b611fe.

See the comments in https://github.com/llvm/llvm-project/pull/84193 for
details

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaExprCXX.cpp

Removed: 
clang/test/SemaCXX/gh84064-1.cpp
clang/test/SemaCXX/gh84064-2.cpp



diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 00b3f53f5c1c66..267c79cc057cba 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6752,18 +6752,10 @@ class Sema final {
 SourceLocation RParenLoc);
 
    ActOnCXXThis -  Parse 'this' pointer.
-  ///
-  /// \param ThisRefersToClosureObject Whether to skip the 'this' check for a
-  /// lambda because 'this' refers to the closure object.
-  ExprResult ActOnCXXThis(SourceLocation loc,
-  bool ThisRefersToClosureObject = false);
+  ExprResult ActOnCXXThis(SourceLocation loc);
 
   /// Build a CXXThisExpr and mark it referenced in the current context.
-  ///
-  /// \param ThisRefersToClosureObject Whether to skip the 'this' check for a
-  /// lambda because 'this' refers to the closure object.
-  Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit,
- bool ThisRefersToClosureObject = false);
+  Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit);
   void MarkThisReferenced(CXXThisExpr *This);
 
   /// Try to retrieve the type of the 'this' pointer.

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 5206fc7621c7cd..736632857efc36 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -25,7 +25,6 @@
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Overload.h"
 #include "clang/Sema/ScopeInfo.h"
-#include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/SmallSet.h"
 
@@ -1291,21 +1290,8 @@ bool CoroutineStmtBuilder::makeReturnOnAllocFailure() {
 static bool collectPlacementArgs(Sema &S, FunctionDecl &FD, SourceLocation Loc,
  SmallVectorImpl &PlacementArgs) {
   if (auto *MD = dyn_cast(&FD)) {
-if (MD->isImplicitObjectMemberFunction()) {
-  ExprResult ThisExpr{};
-
-  if (isLambdaCallOperator(MD) && !MD->isStatic()) {
-Qualifiers ThisQuals = MD->getMethodQualifiers();
-CXXRecordDecl *Record = MD->getParent();
-
-Sema::CXXThisScopeRAII ThisScope(S, Record, ThisQuals,
- Record != nullptr);
-
-ThisExpr = S.ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true);
-  } else {
-ThisExpr = S.ActOnCXXThis(Loc);
-  }
-
+if (MD->isImplicitObjectMemberFunction() && !isLambdaCallOperator(MD)) {
+  ExprResult ThisExpr = S.ActOnCXXThis(Loc);
   if (ThisExpr.isInvalid())
 return false;
   ThisExpr = S.CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 88e3d9ced044cb..c34a40fa7c81ac 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1414,8 +1414,7 @@ bool Sema::CheckCXXThisCapture(SourceLocation Loc, const 
bool Explicit,
   return false;
 }
 
-ExprResult Sema::ActOnCXXThis(SourceLocation Loc,
-  bool ThisRefersToClosureObject) {
+ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
   /// C++ 9.3.2: In the body of a non-static member function, the keyword this
   /// is a non-lvalue expression whose value is the address of the object for
   /// which the function is called.
@@ -1435,18 +1434,13 @@ ExprResult Sema::ActOnCXXThis(SourceLocation Loc,
 return Diag(Loc, diag::err_invalid_this_use) << 0;
   }
 
-  return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false,
-  ThisRefersToClosureObject);
+  return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
 }
 
-Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool 
IsImplicit,
- bool ThisRefersToClosureObject) {
+Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type,
+ bool IsImplicit) {
   auto *This = CXXThisExpr::Create(Context, Loc, Type, IsImplicit);
-
-  if (!ThisRefersToClosureObject) {
-MarkThisReferenced(This);
-  }
-
+  MarkThisReferenced(This);
   return This;
 }
 

diff  --git a/clang/

[clang] Reland "[clang][modules] Print library module manifest path." (PR #82160)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> @ChuanqiXu9 since you suggested this test approach

It looks like that I failed to understand VE is under X86 also...

> do you know whether this is the expected behavior of -sysroot ?

I am not sure. This is surprising to me too.



> The point is to build libraries and test clang at once. The `TC.getFilePaths` 
> is constructed from following items in `ToolChain::ToolChain`
> 
> 1. `BUILD/bin/../lib/x86_64-unknown-linux-gnu`
> 2. `%t/Inputs/usr/lib/x86_64-linux-gnu`
> 3. `%t/Inputs/usr/lib`
> 
> Yes. It contains 1st item at run time if you build libraries. As a result, 
> `GetStdModuleManifestPath` find 
> `BUILD/bin/../lib/x86_64-unknown-linux-gnu/libc++.so` first. However, 
> `modules.json` is at `%t/Inputs/usr/lib/x86_64-linux-gnu/modules.json`.
> 
> Hope this helps you.

It looks like this is not special to VE but reproducible for every constructing 
the clang and libc++ at once. So  I'll try to look at it if I can reproduce it 
locally.

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

[clang] [C++20] [Modules] Introduce a tool 'clang-named-modules-querier' and two plugins 'ClangGetUsedFilesFromModulesPlugin' and 'ClangGetDeclsInModulesPlugin' (PR #72956)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> > ClangGetUsedFilesFromModulesPlugin
> 
> This has a hole where if a currently-unused file is not listed, but it is 
> changed in such a way that it now matters (e.g., it changes include order, 
> adds/removes includes, etc.), we need to recompile consumers.
> 
> > what happens if someone adds an overload, or other interesting name 
> > resolution to the module?
> 
> We would need to do (at least) one of:
> 
> * track considered-but-discarded decls (e.g., if something SFINAE'd away now 
> matters because of a new decl);
> * track "new" decls since the last compile (not sure how the state tracking 
> works here though) and recompile if any show up
> 
> > ```
> >  "col": 12,
> >  "kind": "Function",
> >  "line": 3,
> > ```
> 
> So we change the decl hash if a comment adds a line? That seems like 
> low-hanging fruit to me. Can we enumerate decls and use an index instead? 
> That depends on preprocessor state though, so may be hard to externally 
> verify…

Why I chose to use the location is that in the discourse discussion, it shows 
the location is still meaningful in debug information in generated objects. And 
for index..., I feel it is hard to tell the meaning of the index outside the 
compiler. As you said, how can a consumer be sure that 2 decls with the same 
index in 2 different invocations to be the same one...

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

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

@@ -744,6 +744,9 @@ getCompilationDataBase(int argc, char **argv, std::string 
&ErrorMessage) {
 return nullptr;
   }
 
+  // Only 1 threads is required if P1689 per file mode.
+  NumThreads = 1;

ChuanqiXu9 wrote:

Oh, sorry. I don't know why I missed this somehow. This should be done now.

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

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

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

>From 48b3261e1d217b7ce78180314a222dca4d6aba18 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Thu, 7 Mar 2024 15:19:28 +0800
Subject: [PATCH 1/4] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use
 thread pool in P1689 per file mode

I suddenly found that the clang scan deps may use all concurrent threads
to scan the files. It makes sense in the batch mode. But in P1689
per file mode, it simply wastes times.
---
 clang/tools/clang-scan-deps/ClangScanDeps.cpp | 204 +-
 1 file changed, 108 insertions(+), 96 deletions(-)

diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index d042fecc3dbe63..843816a8ed6515 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -744,6 +744,9 @@ getCompilationDataBase(int argc, char **argv, std::string 
&ErrorMessage) {
 return nullptr;
   }
 
+  // Only 1 threads is required if P1689 per file mode.
+  NumThreads = 1;
+
   // There might be multiple jobs for a compilation. Extract the specified
   // output filename from the last job.
   auto LastCmd = C->getJobs().end();
@@ -867,13 +870,6 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());
 
-  DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
-EagerLoadModules);
-  llvm::DefaultThreadPool Pool(llvm::hardware_concurrency(NumThreads));
-  std::vector> WorkerTools;
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I)
-WorkerTools.push_back(std::make_unique(Service));
-
   std::vector Inputs =
   AdjustingCompilations->getAllCompileCommands();
 
@@ -893,102 +889,118 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   if (Format == ScanningOutputFormat::Full)
 FD.emplace(ModuleName.empty() ? Inputs.size() : 0);
 
-  if (Verbose) {
-llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
- << " files using " << Pool.getMaxConcurrency() << " 
workers\n";
-  }
-
-  llvm::Timer T;
-  T.startTimer();
-
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I) {
-Pool.async([&, I]() {
-  llvm::DenseSet AlreadySeenModules;
-  while (auto MaybeInputIndex = GetNextInputIndex()) {
-size_t LocalIndex = *MaybeInputIndex;
-const tooling::CompileCommand *Input = &Inputs[LocalIndex];
-std::string Filename = std::move(Input->Filename);
-std::string CWD = std::move(Input->Directory);
-
-std::optional MaybeModuleName;
-if (!ModuleName.empty())
-  MaybeModuleName = ModuleName;
-
-std::string OutputDir(ModuleFilesDir);
-if (OutputDir.empty())
-  OutputDir = getModuleCachePath(Input->CommandLine);
-auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
-  return ::lookupModuleOutput(MID, MOK, OutputDir);
-};
-
-// Run the tool on it.
-if (Format == ScanningOutputFormat::Make) {
-  auto MaybeFile =
-  WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
-  if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
- Errs))
-HadErrors = true;
-} else if (Format == ScanningOutputFormat::P1689) {
-  // It is useful to generate the make-format dependency output during
-  // the scanning for P1689. Otherwise the users need to scan again for
-  // it. We will generate the make-format dependency output if we find
-  // `-MF` in the command lines.
-  std::string MakeformatOutputPath;
-  std::string MakeformatOutput;
-
-  auto MaybeRule = WorkerTools[I]->getP1689ModuleDependencyFile(
-  *Input, CWD, MakeformatOutput, MakeformatOutputPath);
-
-  if (handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs))
-HadErrors = true;
+  std::vector> WorkerTools;
 
-  if (!MakeformatOutputPath.empty() && !MakeformatOutput.empty() &&
-  !HadErrors) {
-static std::mutex Lock;
-// With compilation database, we may open different files
-// concurrently or we may write the same file concurrently. So we
-// use a map here to allow multiple compile commands to write to 
the
-// same file. Also we need a lock here to avoid data race.
-static llvm::StringMap OSs;
-std::unique_lock LockGuard(Lock);
-
-auto OSIter = OSs.find(MakeformatOutputPath);
-if (OSIter == OSs.end()) {
-  std::error_code EC;
-  OSIter = OSs.try_emplace(MakeformatOutputPath,
-   MakeformatOutputPath

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

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

>From 48b3261e1d217b7ce78180314a222dca4d6aba18 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Thu, 7 Mar 2024 15:19:28 +0800
Subject: [PATCH 1/5] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use
 thread pool in P1689 per file mode

I suddenly found that the clang scan deps may use all concurrent threads
to scan the files. It makes sense in the batch mode. But in P1689
per file mode, it simply wastes times.
---
 clang/tools/clang-scan-deps/ClangScanDeps.cpp | 204 +-
 1 file changed, 108 insertions(+), 96 deletions(-)

diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index d042fecc3dbe63..843816a8ed6515 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -744,6 +744,9 @@ getCompilationDataBase(int argc, char **argv, std::string 
&ErrorMessage) {
 return nullptr;
   }
 
+  // Only 1 threads is required if P1689 per file mode.
+  NumThreads = 1;
+
   // There might be multiple jobs for a compilation. Extract the specified
   // output filename from the last job.
   auto LastCmd = C->getJobs().end();
@@ -867,13 +870,6 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());
 
-  DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
-EagerLoadModules);
-  llvm::DefaultThreadPool Pool(llvm::hardware_concurrency(NumThreads));
-  std::vector> WorkerTools;
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I)
-WorkerTools.push_back(std::make_unique(Service));
-
   std::vector Inputs =
   AdjustingCompilations->getAllCompileCommands();
 
@@ -893,102 +889,118 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   if (Format == ScanningOutputFormat::Full)
 FD.emplace(ModuleName.empty() ? Inputs.size() : 0);
 
-  if (Verbose) {
-llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
- << " files using " << Pool.getMaxConcurrency() << " 
workers\n";
-  }
-
-  llvm::Timer T;
-  T.startTimer();
-
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I) {
-Pool.async([&, I]() {
-  llvm::DenseSet AlreadySeenModules;
-  while (auto MaybeInputIndex = GetNextInputIndex()) {
-size_t LocalIndex = *MaybeInputIndex;
-const tooling::CompileCommand *Input = &Inputs[LocalIndex];
-std::string Filename = std::move(Input->Filename);
-std::string CWD = std::move(Input->Directory);
-
-std::optional MaybeModuleName;
-if (!ModuleName.empty())
-  MaybeModuleName = ModuleName;
-
-std::string OutputDir(ModuleFilesDir);
-if (OutputDir.empty())
-  OutputDir = getModuleCachePath(Input->CommandLine);
-auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
-  return ::lookupModuleOutput(MID, MOK, OutputDir);
-};
-
-// Run the tool on it.
-if (Format == ScanningOutputFormat::Make) {
-  auto MaybeFile =
-  WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
-  if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
- Errs))
-HadErrors = true;
-} else if (Format == ScanningOutputFormat::P1689) {
-  // It is useful to generate the make-format dependency output during
-  // the scanning for P1689. Otherwise the users need to scan again for
-  // it. We will generate the make-format dependency output if we find
-  // `-MF` in the command lines.
-  std::string MakeformatOutputPath;
-  std::string MakeformatOutput;
-
-  auto MaybeRule = WorkerTools[I]->getP1689ModuleDependencyFile(
-  *Input, CWD, MakeformatOutput, MakeformatOutputPath);
-
-  if (handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs))
-HadErrors = true;
+  std::vector> WorkerTools;
 
-  if (!MakeformatOutputPath.empty() && !MakeformatOutput.empty() &&
-  !HadErrors) {
-static std::mutex Lock;
-// With compilation database, we may open different files
-// concurrently or we may write the same file concurrently. So we
-// use a map here to allow multiple compile commands to write to 
the
-// same file. Also we need a lock here to avoid data race.
-static llvm::StringMap OSs;
-std::unique_lock LockGuard(Lock);
-
-auto OSIter = OSs.find(MakeformatOutputPath);
-if (OSIter == OSs.end()) {
-  std::error_code EC;
-  OSIter = OSs.try_emplace(MakeformatOutputPath,
-   MakeformatOutputPath

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

@@ -893,102 +889,118 @@ int clang_scan_deps_main(int argc, char **argv, const 
llvm::ToolContext &) {
   if (Format == ScanningOutputFormat::Full)
 FD.emplace(ModuleName.empty() ? Inputs.size() : 0);
 
-  if (Verbose) {
-llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
- << " files using " << Pool.getMaxConcurrency() << " 
workers\n";
-  }
-
-  llvm::Timer T;
-  T.startTimer();
-
-  for (unsigned I = 0; I < Pool.getMaxConcurrency(); ++I) {
-Pool.async([&, I]() {
-  llvm::DenseSet AlreadySeenModules;
-  while (auto MaybeInputIndex = GetNextInputIndex()) {
-size_t LocalIndex = *MaybeInputIndex;
-const tooling::CompileCommand *Input = &Inputs[LocalIndex];
-std::string Filename = std::move(Input->Filename);
-std::string CWD = std::move(Input->Directory);
-
-std::optional MaybeModuleName;
-if (!ModuleName.empty())
-  MaybeModuleName = ModuleName;
-
-std::string OutputDir(ModuleFilesDir);
-if (OutputDir.empty())
-  OutputDir = getModuleCachePath(Input->CommandLine);
-auto LookupOutput = [&](const ModuleID &MID, ModuleOutputKind MOK) {
-  return ::lookupModuleOutput(MID, MOK, OutputDir);
-};
-
-// Run the tool on it.
-if (Format == ScanningOutputFormat::Make) {
-  auto MaybeFile =
-  WorkerTools[I]->getDependencyFile(Input->CommandLine, CWD);
-  if (handleMakeDependencyToolResult(Filename, MaybeFile, DependencyOS,
- Errs))
-HadErrors = true;
-} else if (Format == ScanningOutputFormat::P1689) {
-  // It is useful to generate the make-format dependency output during
-  // the scanning for P1689. Otherwise the users need to scan again for
-  // it. We will generate the make-format dependency output if we find
-  // `-MF` in the command lines.
-  std::string MakeformatOutputPath;
-  std::string MakeformatOutput;
-
-  auto MaybeRule = WorkerTools[I]->getP1689ModuleDependencyFile(
-  *Input, CWD, MakeformatOutput, MakeformatOutputPath);
-
-  if (handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs))
-HadErrors = true;
+  std::vector> WorkerTools;

ChuanqiXu9 wrote:

Nice catch. Done.

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

[clang] Reland Print library module manifest path again (PR #84881)

[Chuanqi Xu via cfe-commits]

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

Following of https://github.com/llvm/llvm-project/pull/82160

The reason why the above PR fails is that the `--sysroot` has lower priority 
than the libc++ built from the same source. On the one hand, it matches the 
codes behavior. We will add the built libc++ project paths in the ToolChain 
class. But we will only add the path related to sysroot in Linux class, which 
is derived from the ToolChain classes. So the paths of just built libc++ is in 
the front of the paths relative to sysroot. On the other hand, the behavior 
should be good from the higher level.  Since the just built libc++ has the same 
version number with the just built clang, so it makes sense that these 2 
compilers just matches.

So for patch it self, I hacked it by using resource dir in the test since the 
resource dir has the higher priority, which is not strongly correct since we 
won't do that in practice.

@kaz7 would you like to test on your environment to avoid this get reverted 
again?

On the libc++ side, it shows that it lacks a `modules.json` file for the just 
built libc++ directory. If we don't have that, it will be problematic to use 
std modules from the just built clang and libc++ pair. Then it is not good. And 
I feel it may be problematic for future compiler/standard library developers. 
So I feel this is somewhat  a libc++ issue that need to be fixed.

Also if we don't like the hacked test in the current patch, we must wait for 
libc++ to fix this to proceed. But I feel this is somewhat odd since the test 
of clang shouldn't dependent on libc++.

CC: @mordante 

>From 27665923ce0b9974601d7e5880506e3560bade19 Mon Sep 17 00:00:00 2001
From: Mark de Wever 
Date: Sun, 3 Mar 2024 17:58:08 +0100
Subject: [PATCH 1/2] Reland "[clang][modules] Print library module manifest
 path." (#82160)

This implements a way for the compiler to find the modules.json
associated with the C++23 Standard library modules.

This is based on a discussion in SG15. At the moment no Standard library
installs this manifest. #75741 adds this feature in libc++.

This reverts commit 82f424f766be00b037a706a835d0a0663a2680f1.

Disables the tests on non-X86 platforms as suggested.
---
 clang/include/clang/Driver/Driver.h   | 10 +
 clang/include/clang/Driver/Options.td |  3 ++
 clang/lib/Driver/Driver.cpp   | 44 +++
 ...les-print-library-module-manifest-path.cpp | 40 +
 4 files changed, 97 insertions(+)
 create mode 100644 
clang/test/Driver/modules-print-library-module-manifest-path.cpp

diff --git a/clang/include/clang/Driver/Driver.h 
b/clang/include/clang/Driver/Driver.h
index c4cab360bab3bb..bcf8c1295f2ddf 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -615,6 +615,16 @@ class Driver {
   // FIXME: This should be in CompilationInfo.
   std::string GetProgramPath(StringRef Name, const ToolChain &TC) const;
 
+  /// Lookup the path to the Standard library module manifest.
+  ///
+  /// \param C - The compilation.
+  /// \param TC - The tool chain for additional information on
+  /// directories to search.
+  //
+  // FIXME: This should be in CompilationInfo.
+  std::string GetStdModuleManifestPath(const Compilation &C,
+   const ToolChain &TC) const;
+
   /// HandleAutocompletions - Handle --autocomplete by searching and printing
   /// possible flags, descriptions, and its arguments.
   void HandleAutocompletions(StringRef PassedFlags) const;
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index aca8c9b0d5487a..6e7f142789c994 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5374,6 +5374,9 @@ def print_resource_dir : Flag<["-", "--"], 
"print-resource-dir">,
 def print_search_dirs : Flag<["-", "--"], "print-search-dirs">,
   HelpText<"Print the paths used for finding libraries and programs">,
   Visibility<[ClangOption, CLOption]>;
+def print_std_module_manifest_path : Flag<["-", "--"], 
"print-library-module-manifest-path">,
+  HelpText<"Print the path for the C++ Standard library module manifest">,
+  Visibility<[ClangOption, CLOption]>;
 def print_targets : Flag<["-", "--"], "print-targets">,
   HelpText<"Print the registered targets">,
   Visibility<[ClangOption, CLOption]>;
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 190782a79a2456..eae27769b73ec0 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2196,6 +2196,12 @@ bool Driver::HandleImmediateArgs(const Compilation &C) {
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_std_module_manifest_path)) {
+llvm::outs() << GetStdModuleManifestPath(C, C.getDefaultToolChain())
+ << '\n';
+return false;
+  }
+
   if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) {
 if

[clang] Reland "[clang][modules] Print library module manifest path." (PR #82160)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

I sent the new PR in https://github.com/llvm/llvm-project/pull/84881. See the 
comments there for details.

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

[clang] [NFC] [C++20] [Modules] [P1689] [Scanner] Don't use thread pool in P1689 per file mode (PR #84285)

[Chuanqi Xu via cfe-commits]

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

[clang] 5d7796e - [NFC] [C++20] [Modules] Refactor ReducedBMIGenerator

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-03-13T11:22:32+08:00
New Revision: 5d7796e674224be54c48a8db981f4134845bcc7c

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

LOG: [NFC] [C++20] [Modules] Refactor ReducedBMIGenerator

Changes:
- Don't lookup the emitting module from HeaderSearch. We will use the
  module from the ASTContext directly.
- Remove some useless arguments. Let's addback in the future if
  required.

Added: 


Modified: 
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Serialization/GeneratePCH.cpp

Removed: 




diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index e5db486a71a490..3ed9803fa3745b 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -868,6 +868,8 @@ class PCHGenerator : public SemaConsumer {
 return SemaPtr->getDiagnostics();
   }
 
+  virtual Module *getEmittingModule(ASTContext &Ctx);
+
 public:
   PCHGenerator(const Preprocessor &PP, InMemoryModuleCache &ModuleCache,
StringRef OutputFile, StringRef isysroot,
@@ -887,10 +889,12 @@ class PCHGenerator : public SemaConsumer {
 };
 
 class ReducedBMIGenerator : public PCHGenerator {
+protected:
+  virtual Module *getEmittingModule(ASTContext &Ctx) override;
+
 public:
   ReducedBMIGenerator(const Preprocessor &PP, InMemoryModuleCache &ModuleCache,
-  StringRef OutputFile, std::shared_ptr Buffer,
-  bool IncludeTimestamps);
+  StringRef OutputFile);
 
   void HandleTranslationUnit(ASTContext &Ctx) override;
 };

diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index 50338bfa670f83..81fcd8d5ae9bd3 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -293,11 +293,9 @@ 
GenerateModuleInterfaceAction::CreateOutputFile(CompilerInstance &CI,
 std::unique_ptr
 GenerateReducedModuleInterfaceAction::CreateASTConsumer(CompilerInstance &CI,
 StringRef InFile) {
-  auto Buffer = std::make_shared();
-  return std::make_unique(
-  CI.getPreprocessor(), CI.getModuleCache(),
-  CI.getFrontendOpts().OutputFile, Buffer,
-  /*IncludeTimestamps=*/+CI.getFrontendOpts().IncludeTimestamps);
+  return std::make_unique(CI.getPreprocessor(),
+   CI.getModuleCache(),
+   
CI.getFrontendOpts().OutputFile);
 }
 
 bool GenerateHeaderUnitAction::BeginSourceFileAction(CompilerInstance &CI) {

diff  --git a/clang/lib/Serialization/GeneratePCH.cpp 
b/clang/lib/Serialization/GeneratePCH.cpp
index 2b511b2d5a90a2..f54db36d4a0199 100644
--- a/clang/lib/Serialization/GeneratePCH.cpp
+++ b/clang/lib/Serialization/GeneratePCH.cpp
@@ -41,6 +41,21 @@ PCHGenerator::PCHGenerator(
 PCHGenerator::~PCHGenerator() {
 }
 
+Module *PCHGenerator::getEmittingModule(ASTContext &) {
+  Module *M = nullptr;
+
+  if (PP.getLangOpts().isCompilingModule()) {
+M = PP.getHeaderSearchInfo().lookupModule(PP.getLangOpts().CurrentModule,
+  SourceLocation(),
+  /*AllowSearch*/ false);
+if (!M)
+  assert(PP.getDiagnostics().hasErrorOccurred() &&
+ "emitting module but current module doesn't exist");
+  }
+
+  return M;
+}
+
 void PCHGenerator::HandleTranslationUnit(ASTContext &Ctx) {
   // Don't create a PCH if there were fatal failures during module loading.
   if (PP.getModuleLoader().HadFatalFailure)
@@ -50,16 +65,7 @@ void PCHGenerator::HandleTranslationUnit(ASTContext &Ctx) {
   if (hasErrors && !AllowASTWithErrors)
 return;
 
-  Module *Module = nullptr;
-  if (PP.getLangOpts().isCompilingModule()) {
-Module = PP.getHeaderSearchInfo().lookupModule(
-PP.getLangOpts().CurrentModule, SourceLocation(),
-/*AllowSearch*/ false);
-if (!Module) {
-  assert(hasErrors && "emitting module but current module doesn't exist");
-  return;
-}
-  }
+  Module *Module = getEmittingModule(Ctx);
 
   // Errors that do not prevent the PCH from being written should not cause the
   // overall compilation to fail either.
@@ -84,16 +90,22 @@ ASTDeserializationListener 
*PCHGenerator::GetASTDeserializationListener() {
 
 ReducedBMIGenerator::ReducedBMIGenerator(const Preprocessor &PP,
  InMemoryModuleCache &ModuleCache,
- StringRef OutputFile,
- std::shared_ptr Buffer,
- bool IncludeTimestamp

[clang] 4d62929 - [C++20] [Modules] Disambuguous Clang module and C++20 Named module further

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-03-13T13:57:52+08:00
New Revision: 4d62929852849f768d7397f634cfdebc85de96a4

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

LOG: [C++20] [Modules] Disambuguous Clang module and C++20 Named module further

This patch tries to make the boundary of clang module and C++20 named
module more clear.

The changes included:

- Rename `TranslationUnitKind::TU_Module` to
  `TranslationUnitKind::TU_ClangModule`.
- Rename `Sema::ActOnModuleInclude` to `Sema::ActOnAnnotModuleInclude`.
- Rename `ActOnModuleBegin` to `Sema::ActOnAnnotModuleBegin`.
- Rename `Sema::ActOnModuleEnd` to `Sema::ActOnAnnotModuleEnd`.
- Removes a warning if we're trying to compile a non-module unit as
  C++20 module unit. This is not actually useful and makes (the future)
  implementation unnecessarily complex.

This patch meant to be a NFC fix. But it shows that it fixed a bug
suprisingly that previously we would surppress the unused-value warning
in named modules. Because it shares the same logic with clang modules,
which has headers semantics. This shows the change is meaningful.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Frontend/FrontendActions.h
clang/include/clang/Sema/Sema.h
clang/lib/Parse/Parser.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaModule.cpp
clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
clang/test/Modules/pr72828.cppm

Removed: 
clang/test/Modules/missing-module-declaration.cppm



diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c54105507753eb..d7ab1635cf12bc 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11516,8 +11516,6 @@ def err_module_not_defined : Error<
 def err_module_redeclaration : Error<
   "translation unit contains multiple module declarations">;
 def note_prev_module_declaration : Note<"previous module declaration is here">;
-def err_module_declaration_missing : Error<
-  "missing 'export module' declaration in module interface unit">;
 def err_module_declaration_missing_after_global_module_introducer : Error<
   "missing 'module' declaration at end of global module fragment "
   "introduced here">;

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 862952d336ef31..08fc706e3cbf74 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -560,11 +560,6 @@ class LangOptions : public LangOptionsBase {
 return getCompilingModule() != CMK_None;
   }
 
-  /// Are we compiling a standard c++ module interface?
-  bool isCompilingModuleInterface() const {
-return getCompilingModule() == CMK_ModuleInterface;
-  }
-
   /// Are we compiling a module implementation?
   bool isCompilingModuleImplementation() const {
 return !isCompilingModule() && !ModuleName.empty();
@@ -993,8 +988,8 @@ enum TranslationUnitKind {
   /// not complete.
   TU_Prefix,
 
-  /// The translation unit is a module.
-  TU_Module,
+  /// The translation unit is a clang module.
+  TU_ClangModule,
 
   /// The translation unit is a is a complete translation unit that we might
   /// incrementally extend later.

diff  --git a/clang/include/clang/Frontend/FrontendActions.h 
b/clang/include/clang/Frontend/FrontendActions.h
index 8441af2ee3e718..a620ddfc40447d 100644
--- a/clang/include/clang/Frontend/FrontendActions.h
+++ b/clang/include/clang/Frontend/FrontendActions.h
@@ -125,7 +125,7 @@ class GenerateModuleAction : public ASTFrontendAction {
  StringRef InFile) override;
 
   TranslationUnitKind getTranslationUnitKind() override {
-return TU_Module;
+return TU_ClangModule;
   }
 
   bool hasASTFileSupport() const override { return false; }
@@ -138,7 +138,9 @@ class GenerateInterfaceStubsAction : public 
ASTFrontendAction {
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
  StringRef InFile) override;
 
-  TranslationUnitKind getTranslationUnitKind() override { return TU_Module; }
+  TranslationUnitKind getTranslationUnitKind() override {
+return TU_ClangModule;
+  }
   bool hasASTFileSupport() const override { return false; }
 };
 
@@ -159,6 +161,8 @@ class GenerateModuleInterfaceAction : public 
GenerateModuleAction {
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
  StringRef InFile) override;
 
+  TranslationUnitKind getTranslationUnitKind() override { return TU_Complete; }
+
   std::unique_ptr
   CreateOutputFile(CompilerIns

[clang] [C++20] [Modules] Introduce -fgen-reduced-bmi (PR #85050)

[Chuanqi Xu via cfe-commits]

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

This is the driver part of https://github.com/llvm/llvm-project/pull/75894.

This patch introduces '-fgen-reduced-bmi' to enable generating the reduced BMI.

This patch did:
- When `-fgen-reduced-bmi` is specified but `--precompile` is not specified for 
a module unit, we'll skip the precompile phase to avoid unnecessary  two-phase 
compilation phases. Then if `-c` is specified, we will generate the reduced BMI 
in CodeGenAction as a by-product.
- When `-fgen-reduced-bmi` is specified and `--precompile` is specified, we 
will generate the reduced BMI in GenerateModuleInterfaceAction as a by-product.
- When `-fgen-reduced-bmi` is specified for a non-module unit. We don't do 
anything nor try to give a warn. This is more user friendly so that the end 
users can try to test and experiment with the feature without asking help from 
the build systems.

The core design idea is that users should be able to enable this easily with 
the existing cmake mechanisms.

The future plan for the flag is:
- Add this to clang19 and make it opt-in for 1~2 releases. It depends on the 
testing feedback to decide how long we like to make it opt-in.
- Then we can announce the existing BMI generating may be deprecated and 
suggesting people (end users or build systems) to enable this for 1~2 releases.
- Finally we will enable this by default. When that time comes, the term `BMI` 
will refer to the reduced BMI today and the existing BMI will only be 
meaningful to build systems which loves to support two phase compilations.

I'll send release notes and document  in seperate commits after this get landed.

>From 77c66427c279689256f007cc5706b5f74e47f9e6 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Tue, 12 Mar 2024 17:26:49 +0800
Subject: [PATCH] [C++20] [Modules] Introduce -fgen-reduced-bmi

---
 clang/include/clang/CodeGen/CodeGenAction.h   |  2 +
 clang/include/clang/Driver/Options.td |  6 +++
 .../include/clang/Frontend/FrontendOptions.h  |  9 +++-
 clang/include/clang/Serialization/ASTWriter.h |  7 +--
 clang/lib/CodeGen/CodeGenAction.cpp   | 19 +++
 clang/lib/Driver/Driver.cpp   | 27 +-
 clang/lib/Driver/ToolChains/Clang.cpp | 41 --
 clang/lib/Driver/ToolChains/Clang.h   | 14 +
 clang/lib/Frontend/FrontendActions.cpp|  7 +++
 clang/lib/Frontend/PrecompiledPreamble.cpp|  3 +-
 clang/lib/Serialization/GeneratePCH.cpp   | 23 ++--
 .../test/Driver/module-fgen-reduced-bmi.cppm  | 53 +++
 clang/test/Modules/gen-reduced-bmi.cppm   | 36 +
 13 files changed, 220 insertions(+), 27 deletions(-)
 create mode 100644 clang/test/Driver/module-fgen-reduced-bmi.cppm
 create mode 100644 clang/test/Modules/gen-reduced-bmi.cppm

diff --git a/clang/include/clang/CodeGen/CodeGenAction.h 
b/clang/include/clang/CodeGen/CodeGenAction.h
index 7ad2988e589eb2..186dbb43f01ef7 100644
--- a/clang/include/clang/CodeGen/CodeGenAction.h
+++ b/clang/include/clang/CodeGen/CodeGenAction.h
@@ -57,6 +57,8 @@ class CodeGenAction : public ASTFrontendAction {
   bool loadLinkModules(CompilerInstance &CI);
 
 protected:
+  bool BeginSourceFileAction(CompilerInstance &CI) override;
+
   /// Create a new code generation action.  If the optional \p _VMContext
   /// parameter is supplied, the action uses it without taking ownership,
   /// otherwise it creates a fresh LLVM context and takes ownership.
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index aca8c9b0d5487a..b8ceeff9362df9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3018,6 +3018,7 @@ defm prebuilt_implicit_modules : 
BoolFOption<"prebuilt-implicit-modules",
 
 def fmodule_output_EQ : Joined<["-"], "fmodule-output=">,
   Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>,
+  MarshallingInfoString>,
   HelpText<"Save intermediate module file results when compiling a standard 
C++ module unit.">;
 def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, CC1Option]>,
@@ -3031,6 +3032,11 @@ defm skip_odr_check_in_gmf : BoolOption<"f", 
"skip-odr-check-in-gmf",
   "Perform ODR checks for decls in the global module fragment.">>,
   Group;
 
+def gen_reduced_bmi : Flag<["-"], "fgen-reduced-bmi">,
+  Group, Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Generate the reduced BMI">,
+  MarshallingInfoFlag>;
+
 def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, 
Group,
   Visibility<[ClangOption, CC1Option]>, MetaVarName<"">,
   HelpText<"Specify the interval (in seconds) between attempts to prune the 
module cache">,
diff --git a/clang/include/clang/Frontend/FrontendOptions.h 
b/clang/include/clang/Frontend/FrontendOptions.h
index 8085dbcbf671a6..ddfd4f30d1b773 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h

[clang] [C++20] [Modules] Introduce -fgen-reduced-bmi (PR #85050)

[Chuanqi Xu via cfe-commits]

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

[clang] [C++20] [Modules] Introduce -fgen-reduced-bmi (PR #85050)

[Chuanqi Xu via cfe-commits]

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

>From e5114cdc6a77711dda6a13cbee9cd5cc42be48c6 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Tue, 12 Mar 2024 17:26:49 +0800
Subject: [PATCH] [C++20] [Modules] Introduce -fgen-reduced-bmi

---
 clang/include/clang/CodeGen/CodeGenAction.h   |  2 +
 clang/include/clang/Driver/Options.td |  6 +++
 .../include/clang/Frontend/FrontendOptions.h  |  9 +++-
 clang/include/clang/Serialization/ASTWriter.h |  7 +--
 clang/lib/CodeGen/CodeGenAction.cpp   | 19 +++
 clang/lib/Driver/Driver.cpp   | 27 +-
 clang/lib/Driver/ToolChains/Clang.cpp | 41 --
 clang/lib/Driver/ToolChains/Clang.h   | 14 +
 clang/lib/Frontend/FrontendActions.cpp|  7 +++
 clang/lib/Frontend/PrecompiledPreamble.cpp|  3 +-
 clang/lib/Serialization/GeneratePCH.cpp   | 23 ++--
 .../test/Driver/module-fgen-reduced-bmi.cppm  | 53 +++
 clang/test/Modules/gen-reduced-bmi.cppm   | 36 +
 13 files changed, 220 insertions(+), 27 deletions(-)
 create mode 100644 clang/test/Driver/module-fgen-reduced-bmi.cppm
 create mode 100644 clang/test/Modules/gen-reduced-bmi.cppm

diff --git a/clang/include/clang/CodeGen/CodeGenAction.h 
b/clang/include/clang/CodeGen/CodeGenAction.h
index 7ad2988e589eb2..186dbb43f01ef7 100644
--- a/clang/include/clang/CodeGen/CodeGenAction.h
+++ b/clang/include/clang/CodeGen/CodeGenAction.h
@@ -57,6 +57,8 @@ class CodeGenAction : public ASTFrontendAction {
   bool loadLinkModules(CompilerInstance &CI);
 
 protected:
+  bool BeginSourceFileAction(CompilerInstance &CI) override;
+
   /// Create a new code generation action.  If the optional \p _VMContext
   /// parameter is supplied, the action uses it without taking ownership,
   /// otherwise it creates a fresh LLVM context and takes ownership.
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index aca8c9b0d5487a..b8ceeff9362df9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3018,6 +3018,7 @@ defm prebuilt_implicit_modules : 
BoolFOption<"prebuilt-implicit-modules",
 
 def fmodule_output_EQ : Joined<["-"], "fmodule-output=">,
   Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>,
+  MarshallingInfoString>,
   HelpText<"Save intermediate module file results when compiling a standard 
C++ module unit.">;
 def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, CC1Option]>,
@@ -3031,6 +3032,11 @@ defm skip_odr_check_in_gmf : BoolOption<"f", 
"skip-odr-check-in-gmf",
   "Perform ODR checks for decls in the global module fragment.">>,
   Group;
 
+def gen_reduced_bmi : Flag<["-"], "fgen-reduced-bmi">,
+  Group, Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Generate the reduced BMI">,
+  MarshallingInfoFlag>;
+
 def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, 
Group,
   Visibility<[ClangOption, CC1Option]>, MetaVarName<"">,
   HelpText<"Specify the interval (in seconds) between attempts to prune the 
module cache">,
diff --git a/clang/include/clang/Frontend/FrontendOptions.h 
b/clang/include/clang/Frontend/FrontendOptions.h
index 8085dbcbf671a6..ddfd4f30d1b773 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -387,6 +387,10 @@ class FrontendOptions {
   LLVM_PREFERRED_TYPE(bool)
   unsigned ModulesShareFileManager : 1;
 
+  /// Whether to generate reduced BMI for C++20 named modules.
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned GenReducedBMI : 1;
+
   CodeCompleteOptions CodeCompleteOpts;
 
   /// Specifies the output format of the AST.
@@ -553,6 +557,9 @@ class FrontendOptions {
   /// Path which stores the output files for -ftime-trace
   std::string TimeTracePath;
 
+  /// Output Path for module output file.
+  std::string ModuleOutputPath;
+
 public:
   FrontendOptions()
   : DisableFree(false), RelocatablePCH(false), ShowHelp(false),
@@ -565,7 +572,7 @@ class FrontendOptions {
 BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false),
 IncludeTimestamps(true), UseTemporary(true),
 AllowPCMWithCompilerErrors(false), ModulesShareFileManager(true),
-TimeTraceGranularity(500) {}
+GenReducedBMI(false), TimeTraceGranularity(500) {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
   /// extension. For example, "c" would return Language::C.
diff --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index 3ed9803fa3745b..bd310b6c7a5cdd 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -846,7 +846,7 @@ class ASTWriter : public ASTDeserializationListener,
 /// AST and semantic-analysis consumer that generates a
 /// precompiled header from the 

[clang] [C++20] [Modules] Introduce -fgen-reduced-bmi (PR #85050)

[Chuanqi Xu via cfe-commits]

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

[clang] 2582965 - [C++20] [Modules] [Reduced BMI] Generate the function body from implicitly instantiated class and constant variables

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-03-14T15:07:08+08:00
New Revision: 2582965c160486f9e3b0680f1cebc5ffdef9620c

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

LOG: [C++20] [Modules] [Reduced BMI] Generate the function body from implicitly 
instantiated class and constant variables

After this patch, we will generate the function body from implicitly
instantiated class. This is important for consumers with same
template arguments. Otherwise the consumers won't see the function body.
Since the consumers won't instantiate the templates again if they find an
instantiation.

Also we will generate the variable definition if the variable is
non-inline but known as constant. Such variables may not affect the
ABI, but they may get involved into the compile time constant computation
in the consumer's code. So we have to generate such definitions.

Added: 
clang/test/Modules/reduced-bmi-generating-codes.cppm

Modified: 
clang/lib/Serialization/ASTWriterDecl.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTWriterDecl.cpp 
b/clang/lib/Serialization/ASTWriterDecl.cpp
index d04e1c781b4e28..86f64bf2a24250 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -281,11 +281,18 @@ bool clang::CanElideDeclDef(const Decl *D) {
 
 if (FD->isDependentContext())
   return false;
+
+if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+  return false;
   }
 
   if (auto *VD = dyn_cast(D)) {
 if (!VD->getDeclContext()->getRedeclContext()->isFileContext() ||
-VD->isInline() || VD->isConstexpr() || isa(VD))
+VD->isInline() || VD->isConstexpr() || isa(VD) ||
+// Constant initialized variable may not affect the ABI, but they
+// may be used in constant evaluation in the frontend, so we have
+// to remain them.
+VD->hasConstantInitialization())
   return false;
 
 if (VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)

diff  --git a/clang/test/Modules/reduced-bmi-generating-codes.cppm 
b/clang/test/Modules/reduced-bmi-generating-codes.cppm
new file mode 100644
index 00..13dcda06437b29
--- /dev/null
+++ b/clang/test/Modules/reduced-bmi-generating-codes.cppm
@@ -0,0 +1,40 @@
+// Although the reduced BMI are not designed to be generated,
+// it is helpful for testing whether we've reduced the definitions.
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \
+// RUN: -emit-reduced-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cpp \
+// RUN: -fmodule-file=a=%t/a.pcm -S -emit-llvm -o - \
+// RUN: | FileCheck %t/b.cpp
+
+//--- a.cppm
+export module a;
+
+export template 
+class A {
+public:
+int member() {
+return 43;
+}
+};
+
+// Instantiate `A::member()`.
+export int a_member = A().member();
+
+export const int a = 43;
+
+//--- b.cpp
+import a;
+
+static_assert(a == 43);
+
+int b() {
+A a;
+return a.member();
+}
+
+// CHECK: define{{.*}}@_ZNW1a1AIiE6memberEv



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

[clang] Reland Print library module manifest path again (PR #84881)

[Chuanqi Xu via cfe-commits]

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

[clang] Reland Print library module manifest path again (PR #84881)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

I'll backport this tomorrow if no revert request shows up.

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

[clang] Reland Print library module manifest path again (PR #84881)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> The file we're looking for is `modules.json`; Renaming it 
> `libc++.modules.json` like `.so` / `.a` file might be a better idea which 
> could avoid name clashes when installed in `/usr/lib`.

BTW, how do you feel about the paragraph. I thought it as a defect in some 
level.

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

[clang] Reland Print library module manifest path again (PR #84881)

[Chuanqi Xu via cfe-commits]

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

[clang] Reland Print library module manifest path again (PR #84881)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

/cherry-pick 0e1e1fc8f0e

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

[clang] Reland Print library module manifest path again (PR #84881)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> > > The file we're looking for is `modules.json`; Renaming it 
> > > `libc++.modules.json` like `.so` / `.a` file might be a better idea which 
> > > could avoid name clashes when installed in `/usr/lib`.
> > 
> > 
> > but i didn't rename it, it was with the libc++ prefix directly :/
> 
> Good point I did :-/ It seems we originally talked about `modules.json` and 
> later mentioned using the libname. @ChuanqiXu9 I prefer to keep the current 
> name `libc++.modules.json` and adjust clang. WDYT?

Could you elaborate the reason more? I feel like renaming `libc++.modules.json` 
to `modules.json` is straightforward. Maybe I didn't take part it in the old 
disscussion or I missed : )

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

[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type convertible to `std::coroutine_handle<>` (PR #85684)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> I am also ok with changing the patch to reject programs whose await_suspend 
> doesn't strictly return std::coroutine_handle if upstream prefers.

It should be good to do that since it makes the behavior more conforming. And 
maybe it is better to do this a seperate patch.
Especially if it can help reduce the many non-semantical changes in 
SemaExprCXX.cpp.



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

[clang] Reland Print library module manifest path again (PR #84881)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> > > > > The file we're looking for is `modules.json`; Renaming it 
> > > > > `libc++.modules.json` like `.so` / `.a` file might be a better idea 
> > > > > which could avoid name clashes when installed in `/usr/lib`.
> > > > 
> > > > 
> > > > but i didn't rename it, it was with the libc++ prefix directly :/
> > > 
> > > 
> > > Good point I did :-/ It seems we originally talked about `modules.json` 
> > > and later mentioned using the libname. @ChuanqiXu9 I prefer to keep the 
> > > current name `libc++.modules.json` and adjust clang. WDYT?
> > 
> > 
> > Could you elaborate the reason more? I feel like renaming 
> > `libc++.modules.json` to `modules.json` is straightforward. Maybe I didn't 
> > take part it in the old disscussion or I missed : )
> 
> It would allow to install libstdc++ and libc++ in the same lib directory 
> without "fighting" who owns `modules.json`. Also if we want to provide 
> `libc++-asan.so` in the future we can provide `libc++-asan.modules.json`. 
> Whether we need a different `module.json` for ASan is unknown at the moment, 
> but I can imagine we have additional compiler or linker flags for ASan.

h, I can accept that. Then the patch itself is not incorrect... Since it 
looks like we'd never look for `modules.json` actually, right? Let's adjust the 
implementation in clang.

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

[clang] [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to genera… (PR #90570)

[Chuanqi Xu via cfe-commits]

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

…te module file for C++20 modules instead of PCHGenerator

Previously we're re-using PCHGenerator to generate the module file for C++20 
modules. But this is slighty more or less odd. This patch tries to use a new 
class 'CXX20ModulesGenerator' to generate the module file for C++20 modules.

>From 7a8214efbfc1cc5e16c22bd7e3a21061d5a9555c Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Tue, 30 Apr 2024 13:28:52 +0800
Subject: [PATCH] [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator
 to generate module file for C++20 modules instead of PCHGenerator

Previously we're re-using PCHGenerator to generate the module file for
C++20 modules. But this is slighty more or less odd. This patch tries
to use a new class 'CXX20ModulesGenerator' to generate the module file
for C++20 modules.
---
 clang/include/clang/Serialization/ASTWriter.h | 23 ++---
 clang/lib/Frontend/FrontendActions.cpp| 11 +++-
 clang/lib/Serialization/GeneratePCH.cpp   | 25 +++
 clang/test/Modules/pr67893.cppm   |  2 +-
 clang/test/Modules/search-partitions.cpp  |  8 +++---
 5 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index 6c45b7348b8552..259208b7a91aec 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -885,6 +885,8 @@ class ASTWriter : public ASTDeserializationListener,
 /// AST and semantic-analysis consumer that generates a
 /// precompiled header from the parsed source code.
 class PCHGenerator : public SemaConsumer {
+  void anchor() override;
+
   Preprocessor &PP;
   std::string OutputFile;
   std::string isysroot;
@@ -928,17 +930,32 @@ class PCHGenerator : public SemaConsumer {
   bool hasEmittedPCH() const { return Buffer->IsComplete; }
 };
 
-class ReducedBMIGenerator : public PCHGenerator {
+class CXX20ModulesGenerator : public PCHGenerator {
+  void anchor() override;
 protected:
   virtual Module *getEmittingModule(ASTContext &Ctx) override;
 
+  CXX20ModulesGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
+StringRef OutputFile, bool GeneratingReducedBMI);
+
 public:
-  ReducedBMIGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
-  StringRef OutputFile);
+  CXX20ModulesGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
+StringRef OutputFile)
+  : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
+  /*GeneratingReducedBMI=*/false) {}
 
   void HandleTranslationUnit(ASTContext &Ctx) override;
 };
 
+class ReducedBMIGenerator : public CXX20ModulesGenerator {
+  void anchor() override;
+public:
+  ReducedBMIGenerator(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
+  StringRef OutputFile)
+  : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
+  /*GeneratingReducedBMI=*/true) {}
+};
+
 /// If we can elide the definition of \param D in reduced BMI.
 ///
 /// Generally, we can elide the definition of a declaration if it won't affect
diff --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index 480dfa8c975933..454653a31534cd 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -272,13 +272,10 @@ bool GenerateModuleInterfaceAction::BeginSourceFileAction(
 std::unique_ptr
 GenerateModuleInterfaceAction::CreateASTConsumer(CompilerInstance &CI,
  StringRef InFile) {
-  CI.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true;
-  CI.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
-
-  std::vector> Consumers =
-  CreateMultiplexConsumer(CI, InFile);
-  if (Consumers.empty())
-return nullptr;
+  std::vector> Consumers;
+  Consumers.push_back(std::make_unique(
+  CI.getPreprocessor(), CI.getModuleCache(),
+  CI.getFrontendOpts().OutputFile));
 
   if (CI.getFrontendOpts().GenReducedBMI &&
   !CI.getFrontendOpts().ModuleOutputPath.empty()) {
diff --git a/clang/lib/Serialization/GeneratePCH.cpp 
b/clang/lib/Serialization/GeneratePCH.cpp
index a2ddbe4624aae4..cc06106a47708e 100644
--- a/clang/lib/Serialization/GeneratePCH.cpp
+++ b/clang/lib/Serialization/GeneratePCH.cpp
@@ -88,31 +88,30 @@ ASTDeserializationListener 
*PCHGenerator::GetASTDeserializationListener() {
   return &Writer;
 }
 
-ReducedBMIGenerator::ReducedBMIGenerator(Preprocessor &PP,
- InMemoryModuleCache &ModuleCache,
- StringRef OutputFile)
+void PCHGenerator::anchor() {}
+
+CXX20ModulesGenerator::CXX20ModulesGenerator(Preprocessor &PP,
+ InMemoryModuleCache &ModuleCache,
+   

[clang] [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to genera… (PR #90570)

[Chuanqi Xu via cfe-commits]

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