[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92218)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/92218 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92218)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/92218
Close https://github.com/llvm/llvm-project/issues/91418
Since we load the variable's initializers lazily, it'd be problematic if the
initializers dependent on each other. So here we try to load the initializers
of static variables to make sure they are passed to code generator by order. If
we read any thing interesting, we would consume that before emitting the
current declaration.
>From 699da64855f147708f153c30177a1d02a4e014f7 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 15 May 2024 12:37:16 +0800
Subject: [PATCH] [Serialization] Read the initializer for interesting static
variables before consuming it
Close https://github.com/llvm/llvm-project/issues/91418
Since we load the variable's initializers lazily, it'd be
problematic if the initializers dependent on each other. So here we try to load
the initializers of static variables to make sure they are passed to
code generator by order. If we read any thing interesting, we would
consume that before emitting the current declaration.
---
clang/lib/Serialization/ASTReaderDecl.cpp| 29 ++-
clang/test/Modules/pr91418.cppm | 67 +
clang/test/OpenMP/nvptx_lambda_capturing.cpp | 246 +--
3 files changed, 216 insertions(+), 126 deletions(-)
create mode 100644 clang/test/Modules/pr91418.cppm
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 0c647086e304a..a6254b70560c3 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4186,12 +4186,35 @@ void ASTReader::PassInterestingDeclsToConsumer() {
GetDecl(ID);
EagerlyDeserializedDecls.clear();
- while (!PotentiallyInterestingDecls.empty()) {
-Decl *D = PotentiallyInterestingDecls.front();
-PotentiallyInterestingDecls.pop_front();
+ auto ConsumingPotentialInterestingDecls = [this]() {
+while (!PotentiallyInterestingDecls.empty()) {
+ Decl *D = PotentiallyInterestingDecls.front();
+ PotentiallyInterestingDecls.pop_front();
+ if (isConsumerInterestedIn(D))
+PassInterestingDeclToConsumer(D);
+}
+ };
+ std::deque MaybeInterestingDecls =
+ std::move(PotentiallyInterestingDecls);
+ assert(PotentiallyInterestingDecls.empty());
+ while (!MaybeInterestingDecls.empty()) {
+Decl *D = MaybeInterestingDecls.front();
+MaybeInterestingDecls.pop_front();
+// Since we load the variable's initializers lazily, it'd be problematic
+// if the initializers dependent on each other. So here we try to load the
+// initializers of static variables to make sure they are passed to code
+// generator by order. If we read anything interesting, we would consume
+// that before emitting the current declaration.
+if (auto *VD = dyn_cast(D);
+VD && VD->isFileVarDecl() && !VD->isExternallyVisible())
+ VD->getInit();
+ConsumingPotentialInterestingDecls();
if (isConsumerInterestedIn(D))
PassInterestingDeclToConsumer(D);
}
+
+ // If we add any new potential interesting decl in the last call, consume it.
+ ConsumingPotentialInterestingDecls();
}
void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord ) {
diff --git a/clang/test/Modules/pr91418.cppm b/clang/test/Modules/pr91418.cppm
new file mode 100644
index 0..33fec992439d6
--- /dev/null
+++ b/clang/test/Modules/pr91418.cppm
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -x c++-header
%t/foo.h \
+// RUN: -emit-pch -o %t/foo.pch
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp
-include-pch \
+// RUN: %t/foo.pch -emit-llvm -o - | FileCheck %t/use.cpp
+
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+typedef float __m128 __attribute__((__vector_size__(16), __aligned__(16)));
+
+static __inline__ __m128 __attribute__((__always_inline__,
__min_vector_width__(128)))
+_mm_setr_ps(float __z, float __y, float __x, float __w)
+{
+ return __extension__ (__m128){ __z, __y, __x, __w };
+}
+
+typedef __m128 VR;
+
+inline VR MakeVR( float X, float Y, float Z, float W )
+{
+ return _mm_setr_ps( X, Y, Z, W );
+}
+
+extern "C" float sqrtf(float);
+
+namespace VectorSinConstantsSSE
+{
+ float a = (16 * sqrtf(0.225f));
+ VR A = MakeVR(a, a, a, a);
+ static const float b = (16 * sqrtf(0.225f));
+ static const VR B = MakeVR(b, b, b, b);
+}
+
+#endif // FOO_H
+
+//--- use.cpp
+#include "foo.h"
+float use() {
+return VectorSinConstantsSSE::A[0] + VectorSinConstantsSSE::A[1] +
+ VectorSinConstantsSSE::A[2] + VectorSinConstantsSSE::A[3] +
+ VectorSinConstantsSSE::B[0] + VectorSinConstantsSSE::B[1] +
+ VectorSinConstantsSSE::B[2] + VectorSinConstantsSSE::B[3];
+}
+
+// CHECK: define{{.*}}@__cxx_global_var_init(
+// CHECK: store{{.*}}[[a_RESULT:%[a-zA-Z0-9]+]], ptr
[clang] 11b0591 - [Serialization] Read the initializer for interesting static variables before consuming it
Author: Chuanqi Xu
Date: 2024-05-15T10:29:49+08:00
New Revision: 11b059145d177ee287c7ada9864addf8d083c160
URL:
https://github.com/llvm/llvm-project/commit/11b059145d177ee287c7ada9864addf8d083c160
DIFF:
https://github.com/llvm/llvm-project/commit/11b059145d177ee287c7ada9864addf8d083c160.diff
LOG: [Serialization] Read the initializer for interesting static variables
before consuming it
Close https://github.com/llvm/llvm-project/issues/91418
Since we load the variable's initializers lazily, it'd be problematic
if the initializers dependent on each other. So here we try to load the
initializers of static variables to make sure they are passed to code
generator by order. If we read any thing interesting, we would consume
that before emitting the current declaration.
Added:
clang/test/Modules/pr91418.cppm
Modified:
clang/lib/Serialization/ASTReaderDecl.cpp
Removed:
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 0c647086e304a..a6254b70560c3 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4186,12 +4186,35 @@ void ASTReader::PassInterestingDeclsToConsumer() {
GetDecl(ID);
EagerlyDeserializedDecls.clear();
- while (!PotentiallyInterestingDecls.empty()) {
-Decl *D = PotentiallyInterestingDecls.front();
-PotentiallyInterestingDecls.pop_front();
+ auto ConsumingPotentialInterestingDecls = [this]() {
+while (!PotentiallyInterestingDecls.empty()) {
+ Decl *D = PotentiallyInterestingDecls.front();
+ PotentiallyInterestingDecls.pop_front();
+ if (isConsumerInterestedIn(D))
+PassInterestingDeclToConsumer(D);
+}
+ };
+ std::deque MaybeInterestingDecls =
+ std::move(PotentiallyInterestingDecls);
+ assert(PotentiallyInterestingDecls.empty());
+ while (!MaybeInterestingDecls.empty()) {
+Decl *D = MaybeInterestingDecls.front();
+MaybeInterestingDecls.pop_front();
+// Since we load the variable's initializers lazily, it'd be problematic
+// if the initializers dependent on each other. So here we try to load the
+// initializers of static variables to make sure they are passed to code
+// generator by order. If we read anything interesting, we would consume
+// that before emitting the current declaration.
+if (auto *VD = dyn_cast(D);
+VD && VD->isFileVarDecl() && !VD->isExternallyVisible())
+ VD->getInit();
+ConsumingPotentialInterestingDecls();
if (isConsumerInterestedIn(D))
PassInterestingDeclToConsumer(D);
}
+
+ // If we add any new potential interesting decl in the last call, consume it.
+ ConsumingPotentialInterestingDecls();
}
void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord ) {
diff --git a/clang/test/Modules/pr91418.cppm b/clang/test/Modules/pr91418.cppm
new file mode 100644
index 0..33fec992439d6
--- /dev/null
+++ b/clang/test/Modules/pr91418.cppm
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -x c++-header
%t/foo.h \
+// RUN: -emit-pch -o %t/foo.pch
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp
-include-pch \
+// RUN: %t/foo.pch -emit-llvm -o - | FileCheck %t/use.cpp
+
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+typedef float __m128 __attribute__((__vector_size__(16), __aligned__(16)));
+
+static __inline__ __m128 __attribute__((__always_inline__,
__min_vector_width__(128)))
+_mm_setr_ps(float __z, float __y, float __x, float __w)
+{
+ return __extension__ (__m128){ __z, __y, __x, __w };
+}
+
+typedef __m128 VR;
+
+inline VR MakeVR( float X, float Y, float Z, float W )
+{
+ return _mm_setr_ps( X, Y, Z, W );
+}
+
+extern "C" float sqrtf(float);
+
+namespace VectorSinConstantsSSE
+{
+ float a = (16 * sqrtf(0.225f));
+ VR A = MakeVR(a, a, a, a);
+ static const float b = (16 * sqrtf(0.225f));
+ static const VR B = MakeVR(b, b, b, b);
+}
+
+#endif // FOO_H
+
+//--- use.cpp
+#include "foo.h"
+float use() {
+return VectorSinConstantsSSE::A[0] + VectorSinConstantsSSE::A[1] +
+ VectorSinConstantsSSE::A[2] + VectorSinConstantsSSE::A[3] +
+ VectorSinConstantsSSE::B[0] + VectorSinConstantsSSE::B[1] +
+ VectorSinConstantsSSE::B[2] + VectorSinConstantsSSE::B[3];
+}
+
+// CHECK: define{{.*}}@__cxx_global_var_init(
+// CHECK: store{{.*}}[[a_RESULT:%[a-zA-Z0-9]+]], ptr
@_ZN21VectorSinConstantsSSE1aE
+
+// CHECK: define{{.*}}@__cxx_global_var_init.1(
+// CHECK: [[A_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
+// CHECK: store{{.*}}[[A_CALL]], ptr @_ZN21VectorSinConstantsSSE1AE
+
+// CHECK: define{{.*}}@__cxx_global_var_init.2(
+// CHECK: [[B_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
+// CHECK: store{{.*}}[[B_CALL]], ptr @_ZN21VectorSinConstantsSSEL1BE
+
+// CHECK:
[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
@@ -206,14 +210,37 @@ static void lowerAwaitSuspend(IRBuilder<> ,
CoroAwaitSuspendInst *CB) {
llvm_unreachable("Unexpected coro_await_suspend invocation method");
}
+ if (CB->getCalledFunction()->getIntrinsicID() ==
+ Intrinsic::coro_await_suspend_handle) {
+// Follow the await_suspend by a lowered resume call to the returned
+// coroutine.
+if (auto *Invoke = dyn_cast(CB))
+ Builder.SetInsertPoint(Invoke->getNormalDest()->getFirstInsertionPt());
ChuanqiXu9 wrote:
Oh, got it. Maybe it might be better to make it more clear.
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] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
@@ -206,14 +210,37 @@ static void lowerAwaitSuspend(IRBuilder<> ,
CoroAwaitSuspendInst *CB) {
llvm_unreachable("Unexpected coro_await_suspend invocation method");
}
+ if (CB->getCalledFunction()->getIntrinsicID() ==
+ Intrinsic::coro_await_suspend_handle) {
+// Follow the await_suspend by a lowered resume call to the returned
+// coroutine.
+if (auto *Invoke = dyn_cast(CB))
+ Builder.SetInsertPoint(Invoke->getNormalDest()->getFirstInsertionPt());
ChuanqiXu9 wrote:
What if the CB is a call inst? It may be possible if the exception are disabled.
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] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
https://github.com/ChuanqiXu9 approved this pull request. LGTM with a comment. 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] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
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)
ChuanqiXu9 wrote: @rjmccall @dwblaikie Since I feel this is a bug we need to fix and I think this patch may not affect non-modules code, I'd like to land this in the end of July (before the next branching of 19) if no more comments came in. 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] [serialization] no transitive decl change (PR #91914)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/91914
>From 8ec79e4a7fdf0d75030e81d713ac9fe629ee97eb Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Fri, 10 May 2024 15:36:31 +0800
Subject: [PATCH] [serialization] no transitive decl change
---
clang/include/clang/AST/DeclBase.h| 17 +-
clang/include/clang/AST/DeclID.h | 23 ++-
.../include/clang/Serialization/ASTBitCodes.h | 6 +
clang/include/clang/Serialization/ASTReader.h | 36 ++--
.../include/clang/Serialization/ModuleFile.h | 18 +-
.../clang/Serialization/ModuleManager.h | 2 +-
clang/lib/AST/DeclBase.cpp| 34 +++-
clang/lib/Serialization/ASTReader.cpp | 159 ++
clang/lib/Serialization/ASTReaderDecl.cpp | 12 +-
clang/lib/Serialization/ASTWriter.cpp | 7 +-
clang/lib/Serialization/ModuleFile.cpp| 3 +-
.../Modules/no-transitive-decls-change.cppm | 112
12 files changed, 282 insertions(+), 147 deletions(-)
create mode 100644 clang/test/Modules/no-transitive-decls-change.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index e43e812cd9455..4bdf27aa99405 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -701,10 +701,7 @@ class alignas(8) Decl {
/// Set the owning module ID. This may only be called for
/// deserialized Decls.
- void setOwningModuleID(unsigned ID) {
-assert(isFromASTFile() && "Only works on a deserialized declaration");
-*((unsigned*)this - 2) = ID;
- }
+ void setOwningModuleID(unsigned ID);
public:
/// Determine the availability of the given declaration.
@@ -777,19 +774,11 @@ class alignas(8) Decl {
/// Retrieve the global declaration ID associated with this
/// declaration, which specifies where this Decl was loaded from.
- GlobalDeclID getGlobalID() const {
-if (isFromASTFile())
- return (*((const GlobalDeclID *)this - 1));
-return GlobalDeclID();
- }
+ GlobalDeclID getGlobalID() const;
/// Retrieve the global ID of the module that owns this particular
/// declaration.
- unsigned getOwningModuleID() const {
-if (isFromASTFile())
- return *((const unsigned*)this - 2);
-return 0;
- }
+ unsigned getOwningModuleID() const;
private:
Module *getOwningModuleSlow() const;
diff --git a/clang/include/clang/AST/DeclID.h b/clang/include/clang/AST/DeclID.h
index 614ba06b63860..a6e4b31f3a6fb 100644
--- a/clang/include/clang/AST/DeclID.h
+++ b/clang/include/clang/AST/DeclID.h
@@ -19,6 +19,8 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/iterator.h"
+#include
+
namespace clang {
/// Predefined declaration IDs.
@@ -107,12 +109,16 @@ class DeclIDBase {
///
/// DeclID should only be used directly in serialization. All other users
/// should use LocalDeclID or GlobalDeclID.
- using DeclID = uint32_t;
+ using DeclID = uint64_t;
protected:
DeclIDBase() : ID(PREDEF_DECL_NULL_ID) {}
explicit DeclIDBase(DeclID ID) : ID(ID) {}
+ explicit DeclIDBase(unsigned LocalID, unsigned ModuleFileIndex) {
+ID = (DeclID)LocalID | ((DeclID)ModuleFileIndex << 32);
+ }
+
public:
DeclID get() const { return ID; }
@@ -124,6 +130,15 @@ class DeclIDBase {
bool isInvalid() const { return ID == PREDEF_DECL_NULL_ID; }
+ unsigned getModuleFileIndex() const { return ID >> 32; }
+
+ unsigned getLocalDeclIndex() const {
+// Implement it directly instead of calling `llvm::maskTrailingOnes` since
+// we don't want `MathExtras.h` to be inclued here.
+const unsigned Bits = CHAR_BIT * sizeof(DeclID);
+return ID & (DeclID(-1) >> (Bits - 32));
+ }
+
friend bool operator==(const DeclIDBase , const DeclIDBase ) {
return LHS.ID == RHS.ID;
}
@@ -156,6 +171,9 @@ class LocalDeclID : public DeclIDBase {
LocalDeclID(PredefinedDeclIDs ID) : Base(ID) {}
explicit LocalDeclID(DeclID ID) : Base(ID) {}
+ explicit LocalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
+ : Base(LocalID, ModuleFileIndex) {}
+
LocalDeclID ++() {
++ID;
return *this;
@@ -175,6 +193,9 @@ class GlobalDeclID : public DeclIDBase {
GlobalDeclID() : Base() {}
explicit GlobalDeclID(DeclID ID) : Base(ID) {}
+ explicit GlobalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
+ : Base(LocalID, ModuleFileIndex) {}
+
// For DeclIDIterator to be able to convert a GlobalDeclID
// to a LocalDeclID.
explicit operator LocalDeclID() const { return LocalDeclID(this->ID); }
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index d3538e43d3d78..772452e3afc55 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -255,6 +255,12 @@ class DeclOffset {
}
};
+// The unaligned decl ID used in the Blobs of bistreams.
+using unalighed_decl_id_t =
+
[clang] e74a34b - [NFC] [Serialization] Merge IdentID with IdentifierID
Author: Chuanqi Xu
Date: 2024-05-13T14:05:02+08:00
New Revision: e74a34b6932965dfdc182b69f779e5bee551585a
URL:
https://github.com/llvm/llvm-project/commit/e74a34b6932965dfdc182b69f779e5bee551585a
DIFF:
https://github.com/llvm/llvm-project/commit/e74a34b6932965dfdc182b69f779e5bee551585a.diff
LOG: [NFC] [Serialization] Merge IdentID with IdentifierID
In ASTBitCodes.h, there are two type alias for the ID type of
Identifiers with the same underlying type. It is confusing. This patch
tries to merge the `IdentID` to `IdentifierID` to erase such confusion.
Added:
Modified:
clang/include/clang/Frontend/MultiplexConsumer.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTDeserializationListener.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTWriter.h
clang/include/clang/Serialization/ModuleFile.h
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Frontend/MultiplexConsumer.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderInternals.h
clang/lib/Serialization/ASTWriter.cpp
Removed:
diff --git a/clang/include/clang/Frontend/MultiplexConsumer.h
b/clang/include/clang/Frontend/MultiplexConsumer.h
index f29c8e92fded0..4ed0d86d3cdfb 100644
--- a/clang/include/clang/Frontend/MultiplexConsumer.h
+++ b/clang/include/clang/Frontend/MultiplexConsumer.h
@@ -32,7 +32,7 @@ class MultiplexASTDeserializationListener : public
ASTDeserializationListener {
MultiplexASTDeserializationListener(
const std::vector );
void ReaderInitialized(ASTReader *Reader) override;
- void IdentifierRead(serialization::IdentID ID, IdentifierInfo *II) override;
+ void IdentifierRead(serialization::IdentifierID ID, IdentifierInfo *II)
override;
void MacroRead(serialization::MacroID ID, MacroInfo *MI) override;
void TypeRead(serialization::TypeIdx Idx, QualType T) override;
void DeclRead(GlobalDeclID ID, const Decl *D) override;
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index ae9521e427099..d3538e43d3d78 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -61,6 +61,9 @@ const unsigned VERSION_MINOR = 1;
/// and start at 1. 0 is reserved for NULL.
using IdentifierID = uint32_t;
+/// The number of predefined identifier IDs.
+const unsigned int NUM_PREDEF_IDENT_IDS = 1;
+
/// An ID number that refers to a declaration in an AST file. See the comments
/// in DeclIDBase for details.
using DeclID = DeclIDBase::DeclID;
@@ -123,12 +126,6 @@ struct UnsafeQualTypeDenseMapInfo {
}
};
-/// An ID number that refers to an identifier in an AST file.
-using IdentID = uint32_t;
-
-/// The number of predefined identifier IDs.
-const unsigned int NUM_PREDEF_IDENT_IDS = 1;
-
/// An ID number that refers to a macro in an AST file.
using MacroID = uint32_t;
diff --git a/clang/include/clang/Serialization/ASTDeserializationListener.h
b/clang/include/clang/Serialization/ASTDeserializationListener.h
index 3ab7f1a91843b..1d81a9ae3fe2e 100644
--- a/clang/include/clang/Serialization/ASTDeserializationListener.h
+++ b/clang/include/clang/Serialization/ASTDeserializationListener.h
@@ -35,7 +35,7 @@ class ASTDeserializationListener {
virtual void ReaderInitialized(ASTReader *Reader) { }
/// An identifier was deserialized from the AST file.
- virtual void IdentifierRead(serialization::IdentID ID,
+ virtual void IdentifierRead(serialization::IdentifierID ID,
IdentifierInfo *II) { }
/// A macro was read from the AST file.
virtual void MacroRead(serialization::MacroID ID, MacroInfo *MI) { }
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index e24fa121528f3..1bb5fa27a2419 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -667,7 +667,7 @@ class ASTReader
std::vector IdentifiersLoaded;
using GlobalIdentifierMapType =
- ContinuousRangeMap;
+ ContinuousRangeMap;
/// Mapping from global identifier IDs to the module in which the
/// identifier resides along with the offset that should be added to the
diff --git a/clang/include/clang/Serialization/ASTWriter.h
b/clang/include/clang/Serialization/ASTWriter.h
index 482e9dd168cc3..7bb0e81545bd1 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -277,10 +277,10 @@ class ASTWriter : public ASTDeserializationListener,
std::vector TypeOffsets;
/// The first ID number we can use for our own identifiers.
- serialization::IdentID FirstIdentID = serialization::NUM_PREDEF_IDENT_IDS;
+ serialization::IdentifierID FirstIdentID =
serialization::NUM_PREDEF_IDENT_IDS;
[clang] [serialization] no transitive decl change (PR #91914)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/91914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #91914)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/91914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #91914)
https://github.com/ChuanqiXu9 ready_for_review https://github.com/llvm/llvm-project/pull/91914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #91914)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/91914
Following of https://github.com/llvm/llvm-project/pull/86912
Motivation Example
The motivation of the patch series is that, for a module interface unit `X`,
when the dependent modules of `X` changes, if the changes is not relevant with
`X`, we hope the BMI of `X` won't change. For the specific patch, we hope if
the changes was about irrelevant declaration changes, we hope the BMI of `X`
won't change. **However**, I found the patch itself is not very useful in
practice, since the adding or removing declarations, will change the state of
identifiers and types in most cases.
That said, for the most simple example,
```
// partA.cppm
export module m:partA;
// partA.v1.cppm
export module m:partA;
export void a() {}
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
the BMI of `onlyUseB` will change after we change the implementation of
`partA.cppm` to `partA.v1.cppm`. Since `partA.v1.cppm` introduces new
identifiers and types (the function prototype).
So in this patch, we have to write the tests as:
```
// partA.cppm
export module m:partA;
export int getA() { ... }
export int getA2(int) { ... }
// partA.v1.cppm
export module m:partA;
export int getA() { ... }
export int getA(int) { ... }
export int getA2(int) { ... }
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
so that the new introduced declaration `int getA(int)` doesn't introduce new
identifiers and types, then the BMI of `onlyUseB` can keep unchanged.
While it looks not so great, the patch should be the base of the patch to erase
the transitive change for identifiers and types since I don't know how can we
introduce new types and identifiers without introducing new declarations.
Design details
The design of the patch is similar to
https://github.com/llvm/llvm-project/pull/86912, which extends the 32-bit
DeclID to 64-bit and use the higher bits to store the module file index and the
lower bits to store the Local Decl ID.
A slight difference is that we only use 48 bits to store the new DeclID since
we try to use the higher 16 bits to store the module ID in the prefix of Decl
class. Previously, we use 32 bits to store the module ID and 32 bits to store
the DeclID. I don't want to allocate additional space so I tried to make the
additional space the same as 64 bits. An potential interesting thing here is
about the relationship between the module ID and the module file index. I feel
we can get the module file index by the module ID. But I didn't prove it or
implement it. Since I want to make the patch itself as small as possible. We
can make it in the future if we want.
Another change in the patch is the new concept Decl Index, which means the
index of the very big array `DeclsLoaded` in ASTReader. Previously, the index
of a loaded declaration is simply the Decl ID minus PREDEFINED_DECL_NUMs. So
there are some places they got used ambiguously. But this patch tried to split
these two concepts.
Overhead
As https://github.com/llvm/llvm-project/pull/86912 did, the change will
increase the on-disk PCM file sizes. As the declaration ID may be the most IDs
in the PCM file, this can have the biggest impact on the size. In my
experiments, this change will bring 6.6% increase of the on-disk PCM size. No
compile-time performance regression observed. Given the benefits in the
motivation example, I think the cost is worthwhile.
>From ea53cb5687dd5f3597457fb4d2d62c52c2cb2771 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Fri, 10 May 2024 15:36:31 +0800
Subject: [PATCH] [serialization] no transitive decl change
---
clang/include/clang/AST/DeclBase.h| 17 +-
clang/include/clang/AST/DeclID.h | 23 ++-
.../include/clang/Serialization/ASTBitCodes.h | 6 +
clang/include/clang/Serialization/ASTReader.h | 36 ++--
.../include/clang/Serialization/ModuleFile.h | 18 +-
.../clang/Serialization/ModuleManager.h | 2 +-
clang/lib/AST/DeclBase.cpp| 34 +++-
clang/lib/Serialization/ASTReader.cpp | 159 ++
clang/lib/Serialization/ASTReaderDecl.cpp | 12 +-
clang/lib/Serialization/ASTWriter.cpp | 7 +-
clang/lib/Serialization/ModuleFile.cpp| 3 +-
.../Modules/no-transitive-decls-change.cppm | 112
12 files changed, 282 insertions(+), 147 deletions(-)
create mode 100644 clang/test/Modules/no-transitive-decls-change.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index e43e812cd9455..4bdf27aa99405 100644
---
[clang] [Coro] Relax a debug-info test (PR #91401)
https://github.com/ChuanqiXu9 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/91401 ___ 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)
https://github.com/ChuanqiXu9 approved this pull request. LGTM, thanks : ) 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)
@@ -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)
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)
@@ -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: Yeah, I think so. 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] ad9f38d - [NFC] Fix Modules/no-transitive-source-location-change.cppm after dfa7ff97b2
Author: Chuanqi Xu Date: 2024-05-07T13:25:42+08:00 New Revision: ad9f38d0e3a5e7e06c39dbd7da88a921a49aa805 URL: https://github.com/llvm/llvm-project/commit/ad9f38d0e3a5e7e06c39dbd7da88a921a49aa805 DIFF: https://github.com/llvm/llvm-project/commit/ad9f38d0e3a5e7e06c39dbd7da88a921a49aa805.diff LOG: [NFC] Fix Modules/no-transitive-source-location-change.cppm after dfa7ff97b2 The test fails after dfa7ff97b2. I didn't find this locally due to cache. Added: Modified: clang/test/Modules/no-transitive-source-location-change.cppm Removed: diff --git a/clang/test/Modules/no-transitive-source-location-change.cppm b/clang/test/Modules/no-transitive-source-location-change.cppm index c9d156a74ce822..2a84ef6a912f54 100644 --- a/clang/test/Modules/no-transitive-source-location-change.cppm +++ b/clang/test/Modules/no-transitive-source-location-change.cppm @@ -1,6 +1,9 @@ // Testing that adding a new line in a module interface unit won't cause the BMI // of consuming module unit changes. // +// RUN: rm -rf %t +// RUN: split-file %s %t +// // RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm // RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-reduced-module-interface -o %t/A.v1.pcm // ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] dfa7ff9 - [C++20] [Modules] [Reduced BMI] Combine the signature of used modules
Author: Chuanqi Xu
Date: 2024-05-07T11:41:08+08:00
New Revision: dfa7ff97b24dc5a3dd714b45af288812c13d0110
URL:
https://github.com/llvm/llvm-project/commit/dfa7ff97b24dc5a3dd714b45af288812c13d0110
DIFF:
https://github.com/llvm/llvm-project/commit/dfa7ff97b24dc5a3dd714b45af288812c13d0110.diff
LOG: [C++20] [Modules] [Reduced BMI] Combine the signature of used modules
into the current module
Following of https://github.com/llvm/llvm-project/pull/86912. After
https://github.com/llvm/llvm-project/pull/86912, with reduced BMI, the
BMI can keep unchange if the dependent modules only changes the
implementation (without introduing new decls). However, this is not
strictly correct.
For example:
```
// a.cppm
export module a;
export inline int a() { ... }
// b.cppm
export module b;
import a;
export inline int b() { return a(); }
```
Since both `a()` and `b()` are inline, we need to make sure the BMI of
`b.pcm` will change after the implementation of `a()` changes.
We can't get that naturally since we won't record the body of `a()`
during the writing process. We can't reuse ODRHash here since ODRHash
won't calculate the called function recursively. So ODRHash will be
problematic if `a()` calls other inline functions.
Probably we can solve this by a new hash mechanism. But the safety and
efficiency may a problem too. Here we just combine the hash value of the
used modules conservatively.
Added:
clang/test/Modules/function-transitive-change.cppm
Modified:
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/no-transitive-source-location-change.cppm
Removed:
diff --git a/clang/include/clang/Serialization/ASTWriter.h
b/clang/include/clang/Serialization/ASTWriter.h
index 6847c1db39c8ac..482e9dd168cc3d 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -357,6 +357,13 @@ class ASTWriter : public ASTDeserializationListener,
/// contexts.
llvm::DenseMap AnonymousDeclarationNumbers;
+ /// The external top level module during the writing process. Used to
+ /// generate signature for the module file being written.
+ ///
+ /// Only meaningful for standard C++ named modules. See the comments in
+ /// createSignatureForNamedModule() for details.
+ llvm::DenseSet TouchedTopLevelModules;
+
/// An update to a Decl.
class DeclUpdate {
/// A DeclUpdateKind.
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 8a0116fa893247..42da50abdc687c 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1200,6 +1200,31 @@ ASTFileSignature
ASTWriter::createSignatureForNamedModule() const {
for (auto [ExportImported, _] : WritingModule->Exports)
Hasher.update(ExportImported->Signature);
+ // We combine all the used modules to make sure the signature is precise.
+ // Consider the case like:
+ //
+ // // a.cppm
+ // export module a;
+ // export inline int a() { ... }
+ //
+ // // b.cppm
+ // export module b;
+ // import a;
+ // export inline int b() { return a(); }
+ //
+ // Since both `a()` and `b()` are inline, we need to make sure the BMI of
+ // `b.pcm` will change after the implementation of `a()` changes. We can't
+ // get that naturally since we won't record the body of `a()` during the
+ // writing process. We can't reuse ODRHash here since ODRHash won't calculate
+ // the called function recursively. So ODRHash will be problematic if `a()`
+ // calls other inline functions.
+ //
+ // Probably we can solve this by a new hash mechanism. But the safety and
+ // efficiency may a problem too. Here we just combine the hash value of the
+ // used modules conservatively.
+ for (Module *M : TouchedTopLevelModules)
+Hasher.update(M->Signature);
+
return ASTFileSignature::create(Hasher.result());
}
@@ -6112,8 +6137,12 @@ LocalDeclID ASTWriter::GetDeclRef(const Decl *D) {
// If D comes from an AST file, its declaration ID is already known and
// fixed.
- if (D->isFromASTFile())
+ if (D->isFromASTFile()) {
+if (isWritingStdCXXNamedModules() && D->getOwningModule())
+ TouchedTopLevelModules.insert(D->getOwningModule()->getTopLevelModule());
+
return LocalDeclID(D->getGlobalID());
+ }
assert(!(reinterpret_cast(D) & 0x01) && "Invalid decl pointer");
LocalDeclID = DeclIDs[D];
diff --git a/clang/test/Modules/function-transitive-change.cppm
b/clang/test/Modules/function-transitive-change.cppm
new file mode 100644
index 00..cfce669e3a7bc2
--- /dev/null
+++ b/clang/test/Modules/function-transitive-change.cppm
@@ -0,0 +1,94 @@
+// Test that, in C++20 modules reduced BMI, the implementation detail changes
+// in non-inline function may not propagate while the inline function changes
+// can get propagate.
+//
+// RUN: rm -rf %t
[clang] f9d7619 - [ASTContext] Profile Dependently-sized array types that do not have a specified number
Author: Chuanqi Xu
Date: 2024-05-07T10:59:34+08:00
New Revision: f9d76197ff0099502cf001abe3f5310c5bc4532d
URL:
https://github.com/llvm/llvm-project/commit/f9d76197ff0099502cf001abe3f5310c5bc4532d
DIFF:
https://github.com/llvm/llvm-project/commit/f9d76197ff0099502cf001abe3f5310c5bc4532d.diff
LOG: [ASTContext] Profile Dependently-sized array types that do not have a
specified number
of elements
Close https://github.com/llvm/llvm-project/issues/91105
The root reason for the issue is that we always generate the
dependently-sized array types which don't specify a number of elements.
The original comment says:
> We do no canonicalization here at all, which is okay
> because they can't be used in most locations.
But now we find the locations.
Added:
clang/test/Modules/pr91105.cppm
Modified:
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Type.cpp
Removed:
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 5f96e86f803a80..91e7a5f67a93d3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3797,33 +3797,33 @@ QualType
ASTContext::getDependentSizedArrayType(QualType elementType,
numElements->isValueDependent()) &&
"Size must be type- or value-dependent!");
+ SplitQualType canonElementType = getCanonicalType(elementType).split();
+
+ void *insertPos = nullptr;
+ llvm::FoldingSetNodeID ID;
+ DependentSizedArrayType::Profile(
+ ID, *this, numElements ? QualType(canonElementType.Ty, 0) : elementType,
+ ASM, elementTypeQuals, numElements);
+
+ // Look for an existing type with these properties.
+ DependentSizedArrayType *canonTy =
+DependentSizedArrayTypes.FindNodeOrInsertPos(ID, insertPos);
+
// Dependently-sized array types that do not have a specified number
// of elements will have their sizes deduced from a dependent
- // initializer. We do no canonicalization here at all, which is okay
- // because they can't be used in most locations.
+ // initializer.
if (!numElements) {
+if (canonTy)
+ return QualType(canonTy, 0);
+
auto *newType = new (*this, alignof(DependentSizedArrayType))
DependentSizedArrayType(elementType, QualType(), numElements, ASM,
elementTypeQuals, brackets);
+DependentSizedArrayTypes.InsertNode(newType, insertPos);
Types.push_back(newType);
return QualType(newType, 0);
}
- // Otherwise, we actually build a new type every time, but we
- // also build a canonical type.
-
- SplitQualType canonElementType = getCanonicalType(elementType).split();
-
- void *insertPos = nullptr;
- llvm::FoldingSetNodeID ID;
- DependentSizedArrayType::Profile(ID, *this,
- QualType(canonElementType.Ty, 0),
- ASM, elementTypeQuals, numElements);
-
- // Look for an existing type with these properties.
- DependentSizedArrayType *canonTy =
-DependentSizedArrayTypes.FindNodeOrInsertPos(ID, insertPos);
-
// If we don't have one, build one.
if (!canonTy) {
canonTy = new (*this, alignof(DependentSizedArrayType))
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 2385c5e02cb269..e31741cd44240d 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -256,7 +256,8 @@ void
DependentSizedArrayType::Profile(llvm::FoldingSetNodeID ,
ID.AddPointer(ET.getAsOpaquePtr());
ID.AddInteger(llvm::to_underlying(SizeMod));
ID.AddInteger(TypeQuals);
- E->Profile(ID, Context, true);
+ if (E)
+E->Profile(ID, Context, true);
}
DependentVectorType::DependentVectorType(QualType ElementType,
diff --git a/clang/test/Modules/pr91105.cppm b/clang/test/Modules/pr91105.cppm
new file mode 100644
index 00..0873962c3773ca
--- /dev/null
+++ b/clang/test/Modules/pr91105.cppm
@@ -0,0 +1,47 @@
+// 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 -fskip-odr-check-in-gmf %t/bar.cppm
-emit-module-interface \
+// RUN: -o %t/bar.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/foo.cc \
+// RUN: -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 -fskip-odr-check-in-gmf %t/bar.cppm
-emit-reduced-module-interface \
+// RUN: -o %t/bar.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/foo.cc \
+// RUN: -fmodule-file=bar=%t/bar.pcm -fsyntax-only -verify
+
+//--- h.hpp
+#pragma once
+
+struct T {
+constexpr T(const char *) {}
+};
+template
+struct
[clang] [Modules] No transitive source location change (PR #86912)
ChuanqiXu9 wrote: I've relanded this in https://github.com/llvm/llvm-project/commit/947b06282324db8fe2784c4054af9de493a876af. Let's see what happens. 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] 947b062 - Reland "[Modules] No transitive source location change (#86912)"
Author: Chuanqi Xu
Date: 2024-05-06T13:35:16+08:00
New Revision: 947b06282324db8fe2784c4054af9de493a876af
URL:
https://github.com/llvm/llvm-project/commit/947b06282324db8fe2784c4054af9de493a876af
DIFF:
https://github.com/llvm/llvm-project/commit/947b06282324db8fe2784c4054af9de493a876af.diff
LOG: Reland "[Modules] No transitive source location change (#86912)"
This relands 6c31104.
The patch was reverted due to incorrectly introduced alignment. And the
patch was re-commited after fixing the alignment issue.
Following off are the original message:
This is part of "no transitive change" patch series, "no transitive
source location change". I talked this with @Bigcheese in the tokyo's
WG21 meeting.
The idea comes from @jyknight posted on LLVM discourse. That for:
```
// A.cppm
export module A;
...
// B.cppm
export module B;
import A;
...
//--- C.cppm
export module C;
import C;
```
Almost every time A.cppm changes, we need to recompile `B`. Due to we
think the source location is significant to the semantics. But it may be
good if we can avoid recompiling `C` if the change from `A` wouldn't
change the BMI of B.
This patch only cares source locations. So let's focus on source
location's example. We can see the full example from the attached test.
```
//--- A.cppm
export module A;
export template
struct C {
T func() {
return T(43);
}
};
export int funcA() {
return 43;
}
//--- A.v1.cppm
export module A;
export template
struct C {
T func() {
return T(43);
}
};
export int funcA() {
return 43;
}
//--- B.cppm
export module B;
import A;
export int funcB() {
return funcA();
}
//--- C.cppm
export module C;
import A;
export void testD() {
C c;
c.func();
}
```
Here the only difference between `A.cppm` and `A.v1.cppm` is that
`A.v1.cppm` has an additional blank line. Then the test shows that two
BMI of `B.cppm`, one specified `-fmodule-file=A=A.pcm` and the other
specified `-fmodule-file=A=A.v1.pcm`, should have the bit-wise same
contents.
However, it is a different story for C, since C instantiates templates
from A, and the instantiation records the source information from module
A, which is different from `A` and `A.v1`, so it is expected that the
BMI `C.pcm` and `C.v1.pcm` can and should differ.
To fully understand the patch, we need to understand how we encodes
source locations and how we serialize and deserialize them.
For source locations, we encoded them as:
```
|
|
| _ base offset of an imported module
|
|
|
|_ base offset of another imported module
|
|
|
|
| ___ 0
```
As the diagram shows, we encode the local (unloaded) source location
from 0 to higher bits. And we allocate the space for source locations
from the loaded modules from high bits to 0. Then the source locations
from the loaded modules will be mapped to our source location space
according to the allocated offset.
For example, for,
```
// a.cppm
export module a;
...
// b.cppm
export module b;
import a;
...
```
Assuming the offset of a source location (let's name the location as
`S`) in a.cppm is 45 and we will record the value `45` into the BMI
`a.pcm`. Then in b.cppm, when we import a, the source manager will
allocate a space for module 'a' (according to the recorded number of
source locations) as the base offset of module 'a' in the current source
location spaces. Let's assume the allocated base offset as 90 in this
example. Then when we want to get the location in the current source
location space for `S`, we can get it simply by adding `45` to `90` to
`135`. Finally we can get the source location for `S` in module B as
`135`.
And when we want to write module `b`, we would also write the source
location of `S` as `135` directly in the BMI. And to clarify the
location `S` comes from module `a`, we also need to record the base
offset of module `a`, 90 in the BMI of `b`.
Then the problem comes. Since the base offset of module 'a' is computed
by the number source locations in module 'a'. In module 'b', the
recorded base offset of module 'a' will change every time the number of
source locations in module 'a' increase or decrease. In other words, the
contents of BMI of B will change every time the number of locations in
module 'a' changes. This is pretty sensitive. Almost every change will
change the number of locations. So this is the problem this patch want
to solve.
Let's continue with the existing design to understand what's going on.
Another interesting case is:
```
// c.cppm
export module c;
import whatever;
import a;
import b;
...
```
In `c.cppm`, when we import `a`, we still need to allocate a base
location offset for it, let's say the value becomes to `200` somehow.
Then when we reach the location `S` recorded in module `b`, we need to
translate it into the current source location space. The solution is
quite simple, we can get it by `135 + (200 - 90) = 245`. In another
word, the offset of a source location in current module can be computed
as
[clang] cc1: Report an error for multiple actions unless separated by -main-file-name (PR #91140)
https://github.com/ChuanqiXu9 approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/91140 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Support `-x` for all languages in CL mode (PR #89772)
https://github.com/ChuanqiXu9 commented: It may be better to add a test with `-x c++-module` within CL mode. https://github.com/llvm/llvm-project/pull/89772 ___ 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)
ChuanqiXu9 wrote: thanks, it is pretty helpful. 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] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)
ChuanqiXu9 wrote: > The paper does not clearly says whether disallow function-like macro is also > needed, but I think disallow function-like macro has the same goal as the > paper. WDYT? @cor3ntin @ChuanqiXu9 > > The wording in the paper said: _No identifier in the pp-module-name or > pp-module-partition shall currently be defined as an **object-like macro**._ The intention of the paper is, we can get the module name of a TU by `cat and grep`. So I feel the current wording is correct: https://godbolt.org/z/45xnsh7Eh https://github.com/llvm/llvm-project/pull/90574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] No transitive source location change (PR #86912)
ChuanqiXu9 wrote: Oh, maybe I found the reason. It is because my patch breaks the alignments of `DeclOffset`: https://github.com/llvm/llvm-project/blob/8d28e5861f8b117a547850ffbb9a332aa6e91459/clang/include/clang/Serialization/ASTBitCodes.h#L237-L240 then it explains why it work well in some platforms but not in other platforms. 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] [Modules] No transitive source location change (PR #86912)
ChuanqiXu9 wrote: > > I'll revert this. Due to I can't reproduce this. When the bot gets stable, > > please tell if it is the real problem. > > You can reproduce this: the [GCC compile farm](https://portal.cfarm.net/) > does have a Solaris/sparcv9 system (`cfarm215`) which is perfectly equipped > to run LLVM builds (I've tried). > > I think the stack traces from the bot are a pretty strong indication that > your patch is the culprit: > > ``` > Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH > or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): > 0 clang-19 0x0001076d87b8 > llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 36 > 1 clang-19 0x0001076d910c SignalHandler(int) + 896 > 2 libc.so.1 0x7f0c62a8 __sighndlr + 12 > 3 libc.so.1 0x7f0b8b50 call_user_handler + 1024 > 4 libc.so.1 0x7f0b8f10 sigacthandler + 160 > 5 clang-19 0x0001083824e0 > clang::ASTReader::DeclCursorForID(clang::GlobalDeclID, > clang::SourceLocation&) + 168 > 6 clang-19 0x00010838aca0 > clang::ASTReader::ReadDeclRecord(clang::GlobalDeclID) + 48 > 7 clang-19 0x0001082fb4ec > clang::ASTReader::GetDecl(clang::GlobalDeclID) + 232 > 8 clang-19 0x0001082cb820 > clang::ASTReader::SetGloballyVisibleDecls(clang::IdentifierInfo*, > llvm::SmallVectorImpl const&, > llvm::SmallVectorImpl*) + 252 > 9 clang-19 0x0001083144a0 clang::ASTReader::finishPendingActions() + 572 > 10 clang-19 0x000108319e10 clang::ASTReader::FinishedDeserializing() + 92 > 11 clang-19 0x00010830dbf4 clang::ASTReader::get(llvm::StringRef) + 680 > 12 clang-19 0x0001078a84fc clang::IdentifierTable::get(llvm::StringRef) > + 84 > 13 clang-19 0x00010a130fcc clang::Sema::Initialize() + 1208 > 14 clang-19 0x000109fd1814 clang::Parser::Initialize() + 1260 > 15 clang-19 0x000109fccb68 clang::ParseAST(clang::Sema&, bool, bool) + > 556 > 16 clang-19 0x0001081b10d8 clang::ASTFrontendAction::ExecuteAction() + > 248 > 17 clang-19 0x0001081b06f8 clang::FrontendAction::Execute() + 92 > 18 clang-19 0x0001081196c8 > clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 1572 > 19 clang-19 0x0001082b87b8 > clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 684 > 20 clang-19 0x0001048a2980 cc1_main(llvm::ArrayRef, char > const*, void*) + 4296 > 21 clang-19 0x00010489f6f8 ExecuteCC1Tool(llvm::SmallVectorImpl const*>&, llvm::ToolContext const&) + 1184 > 22 clang-19 0x00010489e018 clang_main(int, char**, llvm::ToolContext > const&) + 4424 > 23 clang-19 0x0001048aee0c main + 60 > 24 clang-19 0x00010489c904 _start + 100 > /var/llvm/dist-sparcv9-release-stage2-A-flang-clang18/tools/clang/stage2-bins/tools/clang/test/PCH/Output/opencl-extensions.cl.script: > line 2: 12701 Bus Error > /var/llvm/dist-sparcv9-release-stage2-A-flang-clang18/tools/clang/stage2-bins/bin/clang > -cc1 -internal-isystem > /var/llvm/dist-sparcv9-release-stage2-A-flang-clang18/tools/clang/stage2-bins/lib/clang/19/include > -nostdsysteminc -include-pch > /var/llvm/dist-sparcv9-release-stage2-A-flang-clang18/tools/clang/stage2-bins/tools/clang/test/PCH/Output/opencl-extensions.cl.tmp > -fsyntax-only > /vol/llvm/src/llvm-project/dist/clang/test/PCH/opencl-extensions.cl -triple > spir-unknown-unknown > ``` > > One thing I see immediately that this uses a triple the bot is not configured > to handle. Nonetheless clang shouldn't die with `SIGBUS` in such as case. Reverted. It looks like the configuration isn't in our bots actually. I can't open that site. I need to take another look at the code though. 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] d333a0d - Revert "[Modules] No transitive source location change (#86912)"
Author: Chuanqi Xu
Date: 2024-04-30T22:32:02+08:00
New Revision: d333a0de6829616427182b26923b14d779ce1dbb
URL:
https://github.com/llvm/llvm-project/commit/d333a0de6829616427182b26923b14d779ce1dbb
DIFF:
https://github.com/llvm/llvm-project/commit/d333a0de6829616427182b26923b14d779ce1dbb.diff
LOG: Revert "[Modules] No transitive source location change (#86912)"
This reverts commit 6c3110464bac3600685af9650269b0b2b8669d34.
Required by the post commit comments:
https://github.com/llvm/llvm-project/pull/86912
Added:
Modified:
clang/include/clang/Basic/SourceLocation.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTWriter.h
clang/include/clang/Serialization/ModuleFile.h
clang/include/clang/Serialization/SourceLocationEncoding.h
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/Serialization/ModuleFile.cpp
clang/test/Modules/pr61067.cppm
clang/unittests/Serialization/SourceLocationEncodingTest.cpp
Removed:
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 7a0f5ba8d1270b..00b1e0fa855b7a 100644
--- a/clang/include/clang/Basic/SourceLocation.h
+++ b/clang/include/clang/Basic/SourceLocation.h
@@ -90,7 +90,6 @@ 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 93e971d7e142c3..a8df5a0bda0850 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -23,7 +23,6 @@
#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
@@ -168,38 +167,45 @@ 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.
- RawLocEncoding Begin;
+ SourceLocation::UIntTy Begin;
/// Raw source location of end of range.
- RawLocEncoding End;
+ SourceLocation::UIntTy End;
/// Offset in the AST file relative to ModuleFile::MacroOffsetsBase.
uint32_t BitOffset;
- PPEntityOffset(RawLocEncoding Begin, RawLocEncoding End, uint32_t BitOffset)
- : Begin(Begin), End(End), BitOffset(BitOffset) {}
+ PPEntityOffset(SourceRange R, uint32_t BitOffset)
+ : Begin(R.getBegin().getRawEncoding()), End(R.getEnd().getRawEncoding()),
+BitOffset(BitOffset) {}
+
+ SourceLocation getBegin() const {
+return SourceLocation::getFromRawEncoding(Begin);
+ }
- RawLocEncoding getBegin() const { return Begin; }
- RawLocEncoding getEnd() const { return End; }
+ SourceLocation getEnd() const {
+return SourceLocation::getFromRawEncoding(End);
+ }
};
/// Source range of a skipped preprocessor region
struct PPSkippedRange {
- using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
-
/// Raw source location of beginning of range.
- RawLocEncoding Begin;
+ SourceLocation::UIntTy Begin;
/// Raw source location of end of range.
- RawLocEncoding End;
+ SourceLocation::UIntTy End;
- PPSkippedRange(RawLocEncoding Begin, RawLocEncoding End)
- : Begin(Begin), End(End) {}
+ PPSkippedRange(SourceRange R)
+ : Begin(R.getBegin().getRawEncoding()), End(R.getEnd().getRawEncoding())
{
+ }
- RawLocEncoding getBegin() const { return Begin; }
- RawLocEncoding getEnd() const { return End; }
+ SourceLocation getBegin() const {
+return SourceLocation::getFromRawEncoding(Begin);
+ }
+ SourceLocation getEnd() const {
+return SourceLocation::getFromRawEncoding(End);
+ }
};
/// Offset in the AST file. Use splitted 64-bit integer into low/high
@@ -225,10 +231,8 @@ struct UnderalignedInt64 {
/// Source location and bit offset of a declaration.
struct DeclOffset {
- using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
-
/// Raw source location.
- RawLocEncoding RawLoc = 0;
+ SourceLocation::UIntTy Loc = 0;
/// Offset relative to the start of the DECLTYPES_BLOCK block. Keep
/// structure alignment 32-bit and avoid padding gap because undefined
@@ -236,15 +240,17 @@ struct DeclOffset {
UnderalignedInt64 BitOffset;
DeclOffset() = default;
-
[clang] [Modules] No transitive source location change (PR #86912)
ChuanqiXu9 wrote: > I strongly suspect that this patch badly broke the [Solaris/sparcv9 > buildbot](https://lab.llvm.org/buildbot/#/builders/72/builds/4046): it > introduced more than 1000 failures. > > Please fix or revert. I'll revert this. Due to I can't reproduce this. When the bot gets stable, please tell if it is the real problem. 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] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)
@@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple x86_64-linux-gnu -DTEST=1 -verify +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple x86_64-linux-gnu -DTEST=2 -verify + +module; +export module x; +#include "version.h" +#if TEST == 1 ChuanqiXu9 wrote: https://llvm.org/docs/TestingGuide.html#extra-files Also it is somewhat clearly that split file is better than the sections guarded by #if-#endif. We can find the example by searching `split-file` under clang/test/Modules https://github.com/llvm/llvm-project/pull/90574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)
@@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple x86_64-linux-gnu -DTEST=1 -verify +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -triple x86_64-linux-gnu -DTEST=2 -verify + +module; +export module x; +#include "version.h" +#if TEST == 1 ChuanqiXu9 wrote: we prefer using split-file now. https://github.com/llvm/llvm-project/pull/90574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)
https://github.com/ChuanqiXu9 commented: LGTM otherwise. I'd like to leave this to @cor3ntin https://github.com/llvm/llvm-project/pull/90574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros (PR #90574)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/90574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 10aab63 - [NFC] [tests] Don't try to remove and create the same directory
Author: Chuanqi Xu Date: 2024-04-30T17:08:40+08:00 New Revision: 10aab63c9cb49d3ddfbe2cf8992de433efeef6f1 URL: https://github.com/llvm/llvm-project/commit/10aab63c9cb49d3ddfbe2cf8992de433efeef6f1 DIFF: https://github.com/llvm/llvm-project/commit/10aab63c9cb49d3ddfbe2cf8992de433efeef6f1.diff LOG: [NFC] [tests] Don't try to remove and create the same directory In the test of clang/test/Modules/no-transitive-source-location-change.cppm, there were reports about invalid directory names in windows. The reason may be that we may remove and create the same directory. This patch tries to avoid such patterns for that. Added: Modified: clang/test/Modules/no-transitive-source-location-change.cppm Removed: diff --git a/clang/test/Modules/no-transitive-source-location-change.cppm b/clang/test/Modules/no-transitive-source-location-change.cppm index 83cf6fb4f684d0..303142a1af890b 100644 --- a/clang/test/Modules/no-transitive-source-location-change.cppm +++ b/clang/test/Modules/no-transitive-source-location-change.cppm @@ -3,7 +3,6 @@ // // RUN: rm -rf %t // RUN: split-file %s %t -// RUN: cd %t // // RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm // RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-module-interface -o %t/A.v1.pcm @@ -25,10 +24,6 @@ // RUN: -o %t/C.v1.pcm // RUN: not diff %t/C.v1.pcm %t/C.pcm &> /dev/null // -// RUN: rm -rf %t -// RUN: split-file %s %t -// RUN: cd %t -// // Test again with reduced BMI. // RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm // RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-reduced-module-interface -o %t/A.v1.pcm ___ 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)
https://github.com/ChuanqiXu9 closed 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
[clang] [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to genera… (PR #90570)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/90570
>From d73596affed67978c703c92789de045e9ebf0f6b 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 | 25 ---
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, 44 insertions(+), 27 deletions(-)
diff --git a/clang/include/clang/Serialization/ASTWriter.h
b/clang/include/clang/Serialization/ASTWriter.h
index 6c45b7348b8552..6f64ece9c5a19b 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
std::string OutputFile;
std::string isysroot;
@@ -928,17 +930,34 @@ 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 ) override;
+ CXX20ModulesGenerator(Preprocessor , InMemoryModuleCache ,
+StringRef OutputFile, bool GeneratingReducedBMI);
+
public:
- ReducedBMIGenerator(Preprocessor , InMemoryModuleCache ,
- StringRef OutputFile);
+ CXX20ModulesGenerator(Preprocessor , InMemoryModuleCache ,
+StringRef OutputFile)
+ : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
+ /*GeneratingReducedBMI=*/false) {}
void HandleTranslationUnit(ASTContext ) override;
};
+class ReducedBMIGenerator : public CXX20ModulesGenerator {
+ void anchor() override;
+
+public:
+ ReducedBMIGenerator(Preprocessor , InMemoryModuleCache ,
+ 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 ,
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
}
-ReducedBMIGenerator::ReducedBMIGenerator(Preprocessor ,
- InMemoryModuleCache ,
- StringRef OutputFile)
+void PCHGenerator::anchor() {}
+
+CXX20ModulesGenerator::CXX20ModulesGenerator(Preprocessor ,
+ InMemoryModuleCache ,
+ StringRef OutputFile,
+ bool GeneratingReducedBMI)
: PCHGenerator(
PP, ModuleCache, OutputFile, llvm::StringRef(),
std::make_shared(),
/*Extensions=*/ArrayRef>(),
/*AllowASTWithErrors*/ false, /*IncludeTimestamps=*/false,
[clang] b2b463b - [C++20] [Modules] Add signature to the BMI recording export imported
Author: Chuanqi Xu
Date: 2024-04-30T16:33:34+08:00
New Revision: b2b463bd8f6b21f040b80c4493682cf74f8dced5
URL:
https://github.com/llvm/llvm-project/commit/b2b463bd8f6b21f040b80c4493682cf74f8dced5
DIFF:
https://github.com/llvm/llvm-project/commit/b2b463bd8f6b21f040b80c4493682cf74f8dced5.diff
LOG: [C++20] [Modules] Add signature to the BMI recording export imported
modules
After https://github.com/llvm/llvm-project/pull/86912,
for the following example,
```
export module A;
export import B;
```
The generated BMI of `A` won't change if the source location in `A`
changes. Further, we plan avoid more such changes.
However, it is slightly problematic since `export import` should
propagate all the changes.
So this patch adds a signature to the BMI of C++20 modules so that we
can propagate the changes correctly.
Added:
clang/test/Modules/force-transitive-changes.cppm
Modified:
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/no-transitive-source-location-change.cppm
Removed:
diff --git a/clang/include/clang/Serialization/ASTWriter.h
b/clang/include/clang/Serialization/ASTWriter.h
index 428bf6a5a791b3..921678d278d6e2 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -525,6 +525,7 @@ class ASTWriter : public ASTDeserializationListener,
/// Calculate hash of the pcm content.
std::pair createSignature() const;
+ ASTFileSignature createSignatureForNamedModule() const;
void WriteInputFiles(SourceManager , HeaderSearchOptions );
void WriteSourceManagerBlock(SourceManager ,
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 4d85f6eb10d232..c3fcd1a4df2368 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1174,26 +1174,47 @@ ASTWriter::createSignature() const {
return std::make_pair(ASTBlockHash, Signature);
}
+ASTFileSignature ASTWriter::createSignatureForNamedModule() const {
+ llvm::SHA1 Hasher;
+ Hasher.update(StringRef(Buffer.data(), Buffer.size()));
+
+ assert(WritingModule);
+ assert(WritingModule->isNamedModule());
+
+ // We need to combine all the export imported modules no matter
+ // we used it or not.
+ for (auto [ExportImported, _] : WritingModule->Exports)
+Hasher.update(ExportImported->Signature);
+
+ return ASTFileSignature::create(Hasher.result());
+}
+
+static void BackpatchSignatureAt(llvm::BitstreamWriter ,
+ const ASTFileSignature , uint64_t BitNo) {
+ for (uint8_t Byte : S) {
+Stream.BackpatchByte(BitNo, Byte);
+BitNo += 8;
+ }
+}
+
ASTFileSignature ASTWriter::backpatchSignature() {
+ if (isWritingStdCXXNamedModules()) {
+ASTFileSignature Signature = createSignatureForNamedModule();
+BackpatchSignatureAt(Stream, Signature, SignatureOffset);
+return Signature;
+ }
+
if (!WritingModule ||
!PP->getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent)
return {};
// For implicit modules, write the hash of the PCM as its signature.
-
- auto BackpatchSignatureAt = [&](const ASTFileSignature , uint64_t BitNo) {
-for (uint8_t Byte : S) {
- Stream.BackpatchByte(BitNo, Byte);
- BitNo += 8;
-}
- };
-
ASTFileSignature ASTBlockHash;
ASTFileSignature Signature;
std::tie(ASTBlockHash, Signature) = createSignature();
- BackpatchSignatureAt(ASTBlockHash, ASTBlockHashOffset);
- BackpatchSignatureAt(Signature, SignatureOffset);
+ BackpatchSignatureAt(Stream, ASTBlockHash, ASTBlockHashOffset);
+ BackpatchSignatureAt(Stream, Signature, SignatureOffset);
return Signature;
}
@@ -1210,9 +1231,11 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor
,
RecordData Record;
Stream.EnterSubblock(UNHASHED_CONTROL_BLOCK_ID, 5);
- // For implicit modules, write the hash of the PCM as its signature.
- if (WritingModule &&
- PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) {
+ // For implicit modules and C++20 named modules, write the hash of the PCM as
+ // its signature.
+ if (isWritingStdCXXNamedModules() ||
+ (WritingModule &&
+ PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent)) {
// At this point, we don't know the actual signature of the file or the AST
// block - we're only able to compute those at the end of the serialization
// process. Let's store dummy signatures for now, and replace them with the
@@ -1223,21 +1246,24 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor
,
auto Dummy = ASTFileSignature::createDummy();
SmallString<128> Blob{Dummy.begin(), Dummy.end()};
-auto Abbrev = std::make_shared();
-Abbrev->Add(BitCodeAbbrevOp(AST_BLOCK_HASH));
-Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
-unsigned
[clang] [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to genera… (PR #90570)
ChuanqiXu9 wrote: The test failure looks no related. I'll commit this after formatted. 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
[clang] [Modules] No transitive source location change (PR #86912)
https://github.com/ChuanqiXu9 closed 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] [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to genera… (PR #90570)
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
[clang] [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to genera… (PR #90570)
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
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 ) override;
+ CXX20ModulesGenerator(Preprocessor , InMemoryModuleCache ,
+StringRef OutputFile, bool GeneratingReducedBMI);
+
public:
- ReducedBMIGenerator(Preprocessor , InMemoryModuleCache ,
- StringRef OutputFile);
+ CXX20ModulesGenerator(Preprocessor , InMemoryModuleCache ,
+StringRef OutputFile)
+ : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
+ /*GeneratingReducedBMI=*/false) {}
void HandleTranslationUnit(ASTContext ) override;
};
+class ReducedBMIGenerator : public CXX20ModulesGenerator {
+ void anchor() override;
+public:
+ ReducedBMIGenerator(Preprocessor , InMemoryModuleCache ,
+ 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 ,
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
}
-ReducedBMIGenerator::ReducedBMIGenerator(Preprocessor ,
- InMemoryModuleCache ,
- StringRef OutputFile)
+void PCHGenerator::anchor() {}
+
+CXX20ModulesGenerator::CXX20ModulesGenerator(Preprocessor ,
+ InMemoryModuleCache ,
+ StringRef OutputFile,
+
[clang] ec527b2 - [C++20] [Modules] Don't skip pragma diagnostic mappings
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 ,
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
) {
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] 6b961e2 - Revert "[C++20] [Modules] Don't skip pragma diagnostic mappings"
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 ) override;
- CXX20ModulesGenerator(Preprocessor , InMemoryModuleCache ,
-StringRef OutputFile, bool GeneratingReducedBMI);
-
public:
- CXX20ModulesGenerator(Preprocessor , InMemoryModuleCache ,
-StringRef OutputFile)
- : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
- /*GeneratingReducedBMI=*/false) {}
+ ReducedBMIGenerator(Preprocessor , InMemoryModuleCache ,
+ StringRef OutputFile);
void HandleTranslationUnit(ASTContext ) override;
};
-class ReducedBMIGenerator : public CXX20ModulesGenerator {
-public:
- ReducedBMIGenerator(Preprocessor , InMemoryModuleCache ,
- 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 ,
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
}
-CXX20ModulesGenerator::CXX20ModulesGenerator(Preprocessor ,
- InMemoryModuleCache ,
- StringRef OutputFile,
- bool GeneratingReducedBMI)
+ReducedBMIGenerator::ReducedBMIGenerator(Preprocessor ,
+ InMemoryModuleCache ,
+ 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 *CXX20ModulesGenerator::getEmittingModule(ASTContext ) {
+Module *ReducedBMIGenerator::getEmittingModule(ASTContext ) {
[clang] fb21343 - [C++20] [Modules] Don't skip pragma diagnostic mappings
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 ) {
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] 18268ac - [NFC] [C++20] [Modules] Use new class CXX20ModulesGenerator to generate module file for C++20 modules instead of PCHGenerator
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 ) override;
+ CXX20ModulesGenerator(Preprocessor , InMemoryModuleCache ,
+StringRef OutputFile, bool GeneratingReducedBMI);
+
public:
- ReducedBMIGenerator(Preprocessor , InMemoryModuleCache ,
- StringRef OutputFile);
+ CXX20ModulesGenerator(Preprocessor , InMemoryModuleCache ,
+StringRef OutputFile)
+ : CXX20ModulesGenerator(PP, ModuleCache, OutputFile,
+ /*GeneratingReducedBMI=*/false) {}
void HandleTranslationUnit(ASTContext ) override;
};
+class ReducedBMIGenerator : public CXX20ModulesGenerator {
+public:
+ ReducedBMIGenerator(Preprocessor , InMemoryModuleCache ,
+ 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 ,
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
}
-ReducedBMIGenerator::ReducedBMIGenerator(Preprocessor ,
- InMemoryModuleCache ,
- StringRef OutputFile)
+CXX20ModulesGenerator::CXX20ModulesGenerator(Preprocessor ,
+ InMemoryModuleCache ,
+ 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 ) {
+Module *CXX20ModulesGenerator::getEmittingModule(ASTContext ) {
Module *M = Ctx.getCurrentNamedModule();
assert(M
[clang] 38067c5 - [C++20] [Modules] [Reduced BMI] Avoid force writing static declarations
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 ,
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 ,
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 )
{
// 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-diagnostics
+export module
[clang] [Coroutines][Test] Specify target triple in coro-elide-thinlto (PR #90549)
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] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
@@ -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] 367efa0 - [NFC] [Modules] Avoid scanning the stored decl list twice when replace
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 = 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 *)
+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 =
-while (DeclListNode *Node = Tail->dyn_cast())
- Tail = >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] 487967a - [Modules] Don't replace local declarations with external declaration with lower visibility
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] Revise the modules document for clarity (PR #90237)
@@ -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] Revise the modules document for clarity (PR #90237)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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. +,
[clang] Revise the modules document for clarity (PR #90237)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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] [llvm] [Pipelines] Do not run CoroSplit and CoroCleanup in LTO pre-link pipeline (PR #90310)
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] [Modules] Detect ODR mismatches for enums in non-C++ like in C++. (PR #90298)
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] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one. (PR #83108)
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] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one. (PR #83108)
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 =(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 ,
@@ -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 ) const {
+ return DeclID < Other.DeclID;
+}
+bool operator==(const LazySpecializationInfo ) 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 ) 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(
+ bool
[clang] [Serialization] Load Specializations Lazily (PR #76774)
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)
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] [Modules] No transitive source location change (PR #86912)
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] [Modules] No transitive source location change (PR #86912)
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,
-
[clang] fe47e8f - [NFC] [ASTUnit] [Serialization] Transalte local decl ID to global decl ID before consuming
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 = *getASTContext().getExternalSource();
+ // The module file of the preamble.
+ serialization::ModuleFile =
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] [NFC] Move DeclID from serialization/ASTBitCodes.h to AST/DeclID.h (PR #89873)
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] [NFC] Move DeclID from serialization/ASTBitCodes.h to AST/DeclID.h (PR #89873)
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 );
};
@@ -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
[clang] [NFC] Move DeclID from serialization/ASTBitCodes.h to AST/DeclID.h (PR #89873)
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)
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 );
};
@@ -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] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
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] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
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] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
https://github.com/ChuanqiXu9 commented: The higher level idea looks fine. 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] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
@@ -1523,24 +1442,16 @@ struct SwitchCoroutineSplitter {
createResumeEntryBlock(F, Shape);
auto *ResumeClone =
-createClone(F, ".resume", Shape, CoroCloner::Kind::SwitchResume);
+createClone(F, ".resume", Shape, CoroCloner::Kind::SwitchResume, TTI);
auto *DestroyClone =
-createClone(F, ".destroy", Shape, CoroCloner::Kind::SwitchUnwind);
+createClone(F, ".destroy", Shape, CoroCloner::Kind::SwitchUnwind, TTI);
auto *CleanupClone =
-createClone(F, ".cleanup", Shape, CoroCloner::Kind::SwitchCleanup);
+createClone(F, ".cleanup", Shape, CoroCloner::Kind::SwitchCleanup,
TTI);
postSplitCleanup(*ResumeClone);
postSplitCleanup(*DestroyClone);
postSplitCleanup(*CleanupClone);
-// Adding musttail call to support symmetric transfer.
-// Skip targets which don't support tail call.
-//
-// FIXME: Could we support symmetric transfer effectively without musttail
ChuanqiXu9 wrote:
Maybe we need to remain the FIXME
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] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
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] [clang] coroutine: generate valid mangled name in CodeGenFunction::generateAwaitSuspendWrapper (PR #89731)
https://github.com/ChuanqiXu9 approved this pull request. LGTM. Thanks. https://github.com/llvm/llvm-project/pull/89731 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] b467c6b - [NFC] [Serialization] Turn type alias GlobalDeclID into a class
Author: Chuanqi Xu
Date: 2024-04-23T17:52:58+08:00
New Revision: b467c6b53660dcaa458c2b5d7fbf5f93ee2af910
URL:
https://github.com/llvm/llvm-project/commit/b467c6b53660dcaa458c2b5d7fbf5f93ee2af910
DIFF:
https://github.com/llvm/llvm-project/commit/b467c6b53660dcaa458c2b5d7fbf5f93ee2af910.diff
LOG: [NFC] [Serialization] Turn type alias GlobalDeclID into a class
Succsessor of b8e3b2ad66cf78ad2b. This patch also converts the type
alias GlobalDeclID to a class to improve the readability and type
safety.
Added:
Modified:
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTRecordReader.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTReaderInternals.h
clang/lib/Serialization/ASTWriter.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index ca51a2dff3d57b..dcfa4ac0c19677 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -79,9 +79,71 @@ class LocalDeclID {
DeclID ID;
};
-// FIXME: Turn GlobalDeclID into class so we can have some type safety when
-// we go from local ID to global and vice-versa.
-using GlobalDeclID = DeclID;
+/// Wrapper class for DeclID. This is helpful to not mix the use of LocalDeclID
+/// and GlobalDeclID to improve the type safety.
+class GlobalDeclID {
+public:
+ GlobalDeclID() : ID(0) {}
+ explicit GlobalDeclID(DeclID ID) : ID(ID) {}
+
+ DeclID get() const { return ID; }
+
+ explicit operator DeclID() const { return ID; }
+
+ friend bool operator==(const GlobalDeclID , const GlobalDeclID ) {
+return LHS.ID == RHS.ID;
+ }
+ friend bool operator!=(const GlobalDeclID , const GlobalDeclID ) {
+return LHS.ID != RHS.ID;
+ }
+ // We may sort the global decl ID.
+ friend bool operator<(const GlobalDeclID , const GlobalDeclID ) {
+return LHS.ID < RHS.ID;
+ }
+ friend bool operator>(const GlobalDeclID , const GlobalDeclID ) {
+return LHS.ID > RHS.ID;
+ }
+ friend bool operator<=(const GlobalDeclID , const GlobalDeclID ) {
+return LHS.ID <= RHS.ID;
+ }
+ friend bool operator>=(const GlobalDeclID , const GlobalDeclID ) {
+return LHS.ID >= RHS.ID;
+ }
+
+private:
+ DeclID ID;
+};
+
+/// A helper iterator adaptor to convert the iterators to `SmallVector`
+/// to the iterators to `SmallVector`.
+class GlobalDeclIDIterator
+: public llvm::iterator_adaptor_base {
+public:
+ GlobalDeclIDIterator() : iterator_adaptor_base(nullptr) {}
+
+ GlobalDeclIDIterator(const DeclID *ID) : iterator_adaptor_base(ID) {}
+
+ value_type operator*() const { return GlobalDeclID(*I); }
+
+ bool operator==(const GlobalDeclIDIterator ) const { return I == RHS.I; }
+};
+
+/// A helper iterator adaptor to convert the iterators to
+/// `SmallVector` to the iterators to `SmallVector`.
+class DeclIDIterator
+: public llvm::iterator_adaptor_base {
+public:
+ DeclIDIterator() : iterator_adaptor_base(nullptr) {}
+
+ DeclIDIterator(const GlobalDeclID *ID) : iterator_adaptor_base(ID) {}
+
+ value_type operator*() const { return DeclID(*I); }
+
+ bool operator==(const DeclIDIterator ) const { return I == RHS.I; }
+};
/// An ID number that refers to a type in an AST file.
///
@@ -2169,6 +2231,27 @@ template <> struct
DenseMapInfo {
}
};
+template <> struct DenseMapInfo {
+ using DeclID = clang::serialization::DeclID;
+ using GlobalDeclID = clang::serialization::GlobalDeclID;
+
+ static GlobalDeclID getEmptyKey() {
+return GlobalDeclID(DenseMapInfo::getEmptyKey());
+ }
+
+ static GlobalDeclID getTombstoneKey() {
+return GlobalDeclID(DenseMapInfo::getTombstoneKey());
+ }
+
+ static unsigned getHashValue(const GlobalDeclID ) {
+return DenseMapInfo::getHashValue(Key.get());
+ }
+
+ static bool isEqual(const GlobalDeclID , const GlobalDeclID ) {
+return L == R;
+ }
+};
+
} // namespace llvm
#endif // LLVM_CLANG_SERIALIZATION_ASTBITCODES_H
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 42aecf059907e8..ed917aa1642293 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -504,7 +504,7 @@ class ASTReader
static_assert(std::is_same_v);
using GlobalDeclMapType =
- ContinuousRangeMap;
+ ContinuousRangeMap;
/// Mapping from global declaration IDs to the module in which the
/// declaration resides.
@@ -513,14 +513,14 @@ class ASTReader
using FileOffset = std::pair;
using FileOffsetsTy = SmallVector;
using DeclUpdateOffsetsMap =
- llvm::DenseMap;
+ llvm::DenseMap;
/// Declarations that have modifications residing in a later file
/// in the chain.
[clang] b8e3b2a - [NFC] [Serialization] Turn type alias LocalDeclID into class
Author: Chuanqi Xu
Date: 2024-04-23T16:56:14+08:00
New Revision: b8e3b2ad66cf78ad2b7832577b1d58dc93c5da21
URL:
https://github.com/llvm/llvm-project/commit/b8e3b2ad66cf78ad2b7832577b1d58dc93c5da21
DIFF:
https://github.com/llvm/llvm-project/commit/b8e3b2ad66cf78ad2b7832577b1d58dc93c5da21.diff
LOG: [NFC] [Serialization] Turn type alias LocalDeclID into class
Previously, the LocalDeclID and GlobalDeclID are defined as:
```
using LocalDeclID = DeclID;
using GlobalDeclID = DeclID;
```
This is more or less concerning that we may misuse LocalDeclID and
GlobalDeclID without understanding it. There is also a FIXME saying
this.
This patch tries to turn LocalDeclID into a class to improve the type
safety here.
Added:
Modified:
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index c91a1c1c82edd4..ca51a2dff3d57b 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -69,9 +69,18 @@ using IdentifierID = uint32_t;
/// FIXME: Merge with Decl::DeclID
using DeclID = uint32_t;
-// FIXME: Turn these into classes so we can have some type safety when
+class LocalDeclID {
+public:
+ explicit LocalDeclID(DeclID ID) : ID(ID) {}
+
+ DeclID get() const { return ID; }
+
+private:
+ DeclID ID;
+};
+
+// FIXME: Turn GlobalDeclID into class so we can have some type safety when
// we go from local ID to global and vice-versa.
-using LocalDeclID = DeclID;
using GlobalDeclID = DeclID;
/// An ID number that refers to a type in an AST file.
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index fe9644eaca4916..42aecf059907e8 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1924,7 +1924,7 @@ class ASTReader
Decl *GetExistingDecl(serialization::DeclID ID);
/// Reads a declaration with the given local ID in the given module.
- Decl *GetLocalDecl(ModuleFile , serialization::DeclID LocalID) {
+ Decl *GetLocalDecl(ModuleFile , serialization::LocalDeclID LocalID) {
return GetDecl(getGlobalDeclID(F, LocalID));
}
@@ -1932,7 +1932,7 @@ class ASTReader
///
/// \returns The requested declaration, casted to the given return type.
template
- T *GetLocalDeclAs(ModuleFile , serialization::DeclID LocalID) {
+ T *GetLocalDeclAs(ModuleFile , serialization::LocalDeclID LocalID) {
return cast_or_null(GetLocalDecl(F, LocalID));
}
diff --git a/clang/include/clang/Serialization/ModuleFile.h
b/clang/include/clang/Serialization/ModuleFile.h
index 675e1e9bc355c5..492c35dceb08d4 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -474,7 +474,7 @@ class ModuleFile {
llvm::DenseMap GlobalToLocalDeclIDs;
/// Array of file-level DeclIDs sorted by file.
- const serialization::DeclID *FileSortedDecls = nullptr;
+ const serialization::LocalDeclID *FileSortedDecls = nullptr;
unsigned NumFileSortedDecls = 0;
/// Array of category list location information within this
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 9764fdc6cd2d49..cfb6ab42c36bd7 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -954,14 +954,16 @@ ASTSelectorLookupTrait::ReadData(Selector, const unsigned
char* d,
// Load instance methods
for (unsigned I = 0; I != NumInstanceMethods; ++I) {
if (ObjCMethodDecl *Method = Reader.GetLocalDeclAs(
-F, endian::readNext(d)))
+F,
+LocalDeclID(endian::readNext(d
Result.Instance.push_back(Method);
}
// Load factory methods
for (unsigned I = 0; I != NumFactoryMethods; ++I) {
if (ObjCMethodDecl *Method = Reader.GetLocalDeclAs(
-F, endian::readNext(d)))
+F,
+LocalDeclID(endian::readNext(d
Result.Factory.push_back(Method);
}
@@ -1091,7 +1093,8 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const
internal_key_type& k,
SmallVector DeclIDs;
for (; DataLen > 0; DataLen -= sizeof(DeclID))
DeclIDs.push_back(Reader.getGlobalDeclID(
- F, endian::readNext(d)));
+ F,
+ LocalDeclID(endian::readNext(d;
Reader.SetGloballyVisibleDecls(II, DeclIDs);
}
@@ -1212,7 +1215,7 @@ void
ASTDeclContextNameLookupTrait::ReadDataInto(internal_key_type,
using namespace llvm::support;
for (unsigned NumDecls = DataLen /
[clang] [Modules] No transitive source location change (PR #86912)
ChuanqiXu9 wrote: @jansvoboda11 @Bigcheese ping 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] aac695d - [NFC] [Serialization] Use semantical type 'DeclID' for 'CreateDeserialized'
Author: Chuanqi Xu
Date: 2024-04-19T16:24:47+08:00
New Revision: aac695da42cf48ccb29c2fe495ead564cc913471
URL:
https://github.com/llvm/llvm-project/commit/aac695da42cf48ccb29c2fe495ead564cc913471
DIFF:
https://github.com/llvm/llvm-project/commit/aac695da42cf48ccb29c2fe495ead564cc913471.diff
LOG: [NFC] [Serialization] Use semantical type 'DeclID' for 'CreateDeserialized'
Previously we use 'unsigned' as the type of ID in 'CreateDeserialized'.
And the type of `DeclID` in serialization is 'uint32_t', so there is
minor inconsistency.
Also more importantly, if we want to extend the type of DeclID from
uint32_t to uint64_t, we may be in trouble due to we forgot updating the
a lot of 'CreateDeserialized'.
So this patch tries to use semantical type 'DeclID' for
'*Decl::CreateDeserialized' to make sure it is tightly consistent.
Added:
Modified:
clang/include/clang/AST/Decl.h
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/DeclCXX.h
clang/include/clang/AST/DeclFriend.h
clang/include/clang/AST/DeclObjC.h
clang/include/clang/AST/DeclOpenMP.h
clang/include/clang/AST/DeclTemplate.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/AST/DeclCXX.cpp
clang/lib/AST/DeclFriend.cpp
clang/lib/AST/DeclObjC.cpp
clang/lib/AST/DeclOpenMP.cpp
clang/lib/AST/DeclTemplate.cpp
Removed:
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 0a9c9e17d3f9f9..8b121896d66d15 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -157,7 +157,7 @@ class PragmaCommentDecl final
SourceLocation CommentLoc,
PragmaMSCommentKind CommentKind,
StringRef Arg);
- static PragmaCommentDecl *CreateDeserialized(ASTContext , unsigned ID,
+ static PragmaCommentDecl *CreateDeserialized(ASTContext , DeclID ID,
unsigned ArgSize);
PragmaMSCommentKind getCommentKind() const { return CommentKind; }
@@ -192,7 +192,7 @@ class PragmaDetectMismatchDecl final
SourceLocation Loc, StringRef Name,
StringRef Value);
static PragmaDetectMismatchDecl *
- CreateDeserialized(ASTContext , unsigned ID, unsigned NameValueSize);
+ CreateDeserialized(ASTContext , DeclID ID, unsigned NameValueSize);
StringRef getName() const { return getTrailingObjects(); }
StringRef getValue() const { return getTrailingObjects() + ValueStart;
}
@@ -518,7 +518,7 @@ class LabelDecl : public NamedDecl {
static LabelDecl *Create(ASTContext , DeclContext *DC,
SourceLocation IdentL, IdentifierInfo *II,
SourceLocation GnuLabelL);
- static LabelDecl *CreateDeserialized(ASTContext , unsigned ID);
+ static LabelDecl *CreateDeserialized(ASTContext , DeclID ID);
LabelStmt *getStmt() const { return TheStmt; }
void setStmt(LabelStmt *T) { TheStmt = T; }
@@ -581,7 +581,7 @@ class NamespaceDecl : public NamedDecl, public DeclContext,
IdentifierInfo *Id, NamespaceDecl *PrevDecl,
bool Nested);
- static NamespaceDecl *CreateDeserialized(ASTContext , unsigned ID);
+ static NamespaceDecl *CreateDeserialized(ASTContext , DeclID ID);
using redecl_range = redeclarable_base::redecl_range;
using redecl_iterator = redeclarable_base::redecl_iterator;
@@ -1146,7 +1146,7 @@ class VarDecl : public DeclaratorDecl, public
Redeclarable {
const IdentifierInfo *Id, QualType T,
TypeSourceInfo *TInfo, StorageClass S);
- static VarDecl *CreateDeserialized(ASTContext , unsigned ID);
+ static VarDecl *CreateDeserialized(ASTContext , DeclID ID);
SourceRange getSourceRange() const override LLVM_READONLY;
@@ -1728,7 +1728,7 @@ class ImplicitParamDecl : public VarDecl {
static ImplicitParamDecl *Create(ASTContext , QualType T,
ImplicitParamKind ParamKind);
- static ImplicitParamDecl *CreateDeserialized(ASTContext , unsigned ID);
+ static ImplicitParamDecl *CreateDeserialized(ASTContext , DeclID ID);
ImplicitParamDecl(ASTContext , DeclContext *DC, SourceLocation IdLoc,
const IdentifierInfo *Id, QualType Type,
@@ -1782,7 +1782,7 @@ class ParmVarDecl : public VarDecl {
TypeSourceInfo *TInfo, StorageClass S,
Expr *DefArg);
- static ParmVarDecl *CreateDeserialized(ASTContext , unsigned ID);
+ static ParmVarDecl *CreateDeserialized(ASTContext , DeclID ID);
SourceRange getSourceRange() const override LLVM_READONLY;
@@ -2178,7 +2178,7 @@ class FunctionDecl : public
[clang] [Modules] No transitive source location change (PR #86912)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/86912
>From ddb4074b0460daf7b42531ec62e97347b3f2e14d Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Mon, 18 Mar 2024 08:36:55 +0800
Subject: [PATCH 1/4] [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 | 54 +++-
clang/include/clang/Serialization/ASTWriter.h | 4 +
.../include/clang/Serialization/ModuleFile.h | 4 -
.../Serialization/SourceLocationEncoding.h| 88 +--
clang/lib/Frontend/ASTUnit.cpp| 2 -
clang/lib/Serialization/ASTReader.cpp | 84 +++---
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, 275 insertions(+), 176 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 500098dd3dab1d..eca776a77e4557 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -22,6 +22,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
@@ -175,45 +176,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
@@ -239,8 +233,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
@@ -248,17 +244,15 @@ struct DeclOffset {
UnderalignedInt64 BitOffset;
DeclOffset() = default;
- DeclOffset(SourceLocation Loc, uint64_t BitOffset,
-
[clang] [Modules] No transitive source location change (PR #86912)
@@ -4082,14 +4069,14 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile )
const {
: ModuleMgr.lookupByFileName(Name));
if (!OM) {
std::string Msg =
- "SourceLocation remap refers to unknown module, cannot find ";
+ "cannot find module ";
ChuanqiXu9 wrote:
Done
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] [Modules] No transitive source location change (PR #86912)
@@ -2221,33 +,45 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source location space.
- SourceLocation ReadUntranslatedSourceLocation(SourceLocation::UIntTy Raw,
-LocSeq *Seq = nullptr) const {
+ std::pair
+ ReadUntranslatedSourceLocation(RawLocEncoding Raw,
+ LocSeq *Seq = nullptr) const {
return SourceLocationEncoding::decode(Raw, Seq);
}
/// Read a source location from raw form.
- SourceLocation ReadSourceLocation(ModuleFile ,
-SourceLocation::UIntTy Raw,
-LocSeq *Seq = nullptr) const {
-SourceLocation Loc = ReadUntranslatedSourceLocation(Raw, Seq);
-return TranslateSourceLocation(ModuleFile, Loc);
+ SourceLocation ReadSourceLocation(ModuleFile , RawLocEncoding Raw,
+ LocSeq *Seq = nullptr) const {
+if (!MF.ModuleOffsetMap.empty())
+ ReadModuleOffsetMap(MF);
+
+auto [Loc, ModuleFileIndex] = ReadUntranslatedSourceLocation(Raw, Seq);
+ModuleFile *OwningModuleFile =
+ModuleFileIndex == 0 ? : MF.DependentModules[ModuleFileIndex - 1];
+
+assert(!SourceMgr.isLoadedSourceLocation(Loc) && "Run out source location
space");
ChuanqiXu9 wrote:
But the value of `Loc` may not be valid. Also I feel it is fine to have some
redundant assertions. It helps the reader to understand the codes better.
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] [Modules] No transitive source location change (PR #86912)
@@ -2220,33 +2221,40 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source location space.
- SourceLocation ReadUntranslatedSourceLocation(SourceLocation::UIntTy Raw,
-LocSeq *Seq = nullptr) const {
+ std::pair
+ ReadUntranslatedSourceLocation(RawLocEncoding Raw,
+ LocSeq *Seq = nullptr) const {
return SourceLocationEncoding::decode(Raw, Seq);
}
/// Read a source location from raw form.
- SourceLocation ReadSourceLocation(ModuleFile ,
-SourceLocation::UIntTy Raw,
-LocSeq *Seq = nullptr) const {
-SourceLocation Loc = ReadUntranslatedSourceLocation(Raw, Seq);
-return TranslateSourceLocation(ModuleFile, Loc);
+ SourceLocation ReadSourceLocation(ModuleFile , RawLocEncoding Raw,
+ LocSeq *Seq = nullptr) const {
+if (!MF.ModuleOffsetMap.empty())
+ ReadModuleOffsetMap(MF);
+
+auto [Loc, ModuleFileIndex] = ReadUntranslatedSourceLocation(Raw, Seq);
+ModuleFile *OwningModuleFile =
+ModuleFileIndex ? MF.DependentModules[ModuleFileIndex - 1] :
+return TranslateSourceLocation(*OwningModuleFile, Loc);
}
/// Translate a source location from another module file's source
/// location space into ours.
SourceLocation TranslateSourceLocation(ModuleFile ,
SourceLocation Loc) const {
-if (!ModuleFile.ModuleOffsetMap.empty())
- ReadModuleOffsetMap(ModuleFile);
-assert(ModuleFile.SLocRemap.find(Loc.getOffset()) !=
- ModuleFile.SLocRemap.end() &&
- "Cannot find offset to remap.");
-SourceLocation::IntTy Remap =
-ModuleFile.SLocRemap.find(Loc.getOffset())->second;
-return Loc.getLocWithOffset(Remap);
+if (Loc.isInvalid())
+ return Loc;
+
+// It implies that the Loc is already translated.
+if (SourceMgr.isLoadedSourceLocation(Loc))
+ return Loc;
ChuanqiXu9 wrote:
> Now that TranslateSourceLocation() is only called from ReadSourceLocation()
Sadly, this is not true. `TranslateSourceLocation()` may be called in
`ASTReader::ReadAST()`:
https://github.com/llvm/llvm-project/blob/aac4d03423dd6b7bdef0f2eb03c570f3e2ca6630/clang/lib/Serialization/ASTReader.cpp#L4588-L4591
The input value of `TranslateSourceLocation()` there may come from a reading
of untranslated source location in `ASTReader::ReadControlBlock` when reading
imported modules. Or the input value may come from the argument of
`ASTReader::ReadAST()`, where must be a translated source location.
Then it looks really dangerous to me. So I add the FIXME. We may not be able to
change the signature of the argument of `TranslateSourceLocation()` to
`UntranslatedSourceLocation` since that will require us to change the signature
of `ASTReader::ReadAST()`.
The reason why actually it works, is that, in the case the input value comes
from a translated source location (passed directly in `ASTReader::ReadAST()`),
the value of `M.ImportedBy` may always be null **now**. But I feel it is
dangerous if someone changes it suddenly.
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] aac4d03 - [C++20] [Modules] Mark exported all declarations as used
Author: Chuanqi Xu
Date: 2024-04-19T09:46:18+08:00
New Revision: aac4d03423dd6b7bdef0f2eb03c570f3e2ca6630
URL:
https://github.com/llvm/llvm-project/commit/aac4d03423dd6b7bdef0f2eb03c570f3e2ca6630
DIFF:
https://github.com/llvm/llvm-project/commit/aac4d03423dd6b7bdef0f2eb03c570f3e2ca6630.diff
LOG: [C++20] [Modules] Mark exported all declarations as used
Close https://github.com/llvm/llvm-project/issues/85122
As the title suggested, it looks pretty sensible.
Added:
clang/test/Modules/pr85122.cppm
Modified:
clang/lib/Sema/SemaModule.cpp
Removed:
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 67658c93ed3baf..ad118ac90e4aa6 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -1003,6 +1003,10 @@ Decl *Sema::ActOnFinishExportDecl(Scope *S, Decl *D,
SourceLocation RBraceLoc) {
}
}
+ // Anything exported from a module should never be considered unused.
+ for (auto *Exported : ED->decls())
+Exported->markUsed(getASTContext());
+
return D;
}
diff --git a/clang/test/Modules/pr85122.cppm b/clang/test/Modules/pr85122.cppm
new file mode 100644
index 00..a4c89f13711a36
--- /dev/null
+++ b/clang/test/Modules/pr85122.cppm
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -std=c++20 %s -Wall -fsyntax-only -verify
+
+// expected-no-diagnostics
+export module a;
+
+export constexpr auto a = []{};
___
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)
ChuanqiXu9 wrote: Fix conflicts and rebase with main. @Bigcheese @jansvoboda11 ping~ 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] e7a8dd9 - [docs] [C++20] [Modules] Mentioning Module Initializer
Author: Chuanqi Xu Date: 2024-04-18T16:12:14+08:00 New Revision: e7a8dd9b0d419403fe1d8adeb177a4ec78e036cc URL: https://github.com/llvm/llvm-project/commit/e7a8dd9b0d419403fe1d8adeb177a4ec78e036cc DIFF: https://github.com/llvm/llvm-project/commit/e7a8dd9b0d419403fe1d8adeb177a4ec78e036cc.diff LOG: [docs] [C++20] [Modules] Mentioning Module Initializer Although we want to treat the module initializer as a transparent concept to users, but it shows that people need to understand the concept to understand how to understand and distribute modules. So it is better to mention this too. Added: Modified: clang/docs/StandardCPlusPlusModules.rst Removed: diff --git a/clang/docs/StandardCPlusPlusModules.rst b/clang/docs/StandardCPlusPlusModules.rst index 8d5529d5d37db5..ee57fb5da64857 100644 --- a/clang/docs/StandardCPlusPlusModules.rst +++ b/clang/docs/StandardCPlusPlusModules.rst @@ -483,6 +483,13 @@ violations with the flag enabled. ABI Impacts --- +This section describes the new ABI changes brought by modules. + +Only Itanium C++ ABI related change are mentioned + +Mangling Names +~~ + The declarations in a module unit which are not in the global module fragment have new linkage names. For example, @@ -520,6 +527,23 @@ is attached to the global module fragments. For example: 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. + +It is allowed to omit calls to importing modules if it is known empty. + +It is allowed to omit calls to importing modules for which is known to be called. + Reduced BMI --- ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
