Skip to content

[C++20] [Modules] Don't record implicitly declarations to BMI by default #93459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 28, 2024

Conversation

ChuanqiXu9
Copy link
Member

I found we may insert unused implciit declarations like AArch SVE declarations by default on AArch64 due to we will insert that by default. But it should be completely redundant and this patch tries to remove that.

I found we may insert unused implciit declarations like AArch SVE
declarations by default on AArch64 due to we will insert that by
default. But it should be completely redundant and this patch tries to
remove that.
@ChuanqiXu9 ChuanqiXu9 added clang:modules C++20 modules and Clang Header Modules skip-precommit-approval PR for CI feedback, not intended for review labels May 27, 2024
@ChuanqiXu9 ChuanqiXu9 self-assigned this May 27, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label May 27, 2024
@llvmbot
Copy link
Member

llvmbot commented May 27, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)

Changes

I found we may insert unused implciit declarations like AArch SVE declarations by default on AArch64 due to we will insert that by default. But it should be completely redundant and this patch tries to remove that.


Full diff: https://github.com/llvm/llvm-project/pull/93459.diff

2 Files Affected:

  • (modified) clang/lib/Serialization/ASTWriter.cpp (+11-2)
  • (added) clang/test/Modules/no-implicit-declarations.cppm (+26)
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 00b0e48083217..a85cd94fd5b5a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5037,6 +5037,14 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema &SemaRef) {
         continue;
     }
 
+    // If we're writing C++ named modules, don't emit declarations which are
+    // not from modules by default. They may be built in declarations (be
+    // handled above) or implcit declarations (see the implementation of
+    // `Sema::Initialize()` for example).
+    if (isWritingStdCXXNamedModules() && !D->getOwningModule() &&
+        D->isImplicit())
+      continue;
+
     GetDeclRef(D);
   }
 
@@ -6197,8 +6205,9 @@ bool ASTWriter::wasDeclEmitted(const Decl *D) const {
     return true;
 
   bool Emitted = DeclIDs.contains(D);
-  assert((Emitted || GeneratingReducedBMI) &&
-         "The declaration can only be omitted in reduced BMI.");
+  assert((Emitted || (!D->getOwningModule() && isWritingStdCXXNamedModules()) ||
+          GeneratingReducedBMI) &&
+         "The declaration within modules can only be omitted in reduced BMI.");
   return Emitted;
 }
 
diff --git a/clang/test/Modules/no-implicit-declarations.cppm b/clang/test/Modules/no-implicit-declarations.cppm
new file mode 100644
index 0000000000000..319d3a432ea23
--- /dev/null
+++ b/clang/test/Modules/no-implicit-declarations.cppm
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+//
+// RUN: %clang_cc1 -std=c++20 %s -emit-module-interface -o %t/a.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t/a.pcm > %t/a.dump
+// RUN: cat %t/a.dump | FileCheck %s
+//
+// RUN: %clang_cc1 -std=c++20 %s -emit-reduced-module-interface -o %t/a.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t/a.pcm > %t/a.dump
+// RUN: cat %t/a.dump | FileCheck %s
+
+export module a;
+// Contain something at least to make sure the compiler won't
+// optimize this out.
+export int a = 43;
+
+// CHECK:  <DECLTYPES_BLOCK
+// CHECK-NOT: <DECL_TYPEDEF
+// CHECK:    <DECL_CONTEXT_LEXICAL
+// CHECK:    <UnknownCode
+// CHECK:    <TYPE_TYPEDEF
+// CHECK:    <DECL_VAR
+// CHECK:    <EXPR_INTEGER_LITERAL
+// CHECK:    <STMT_STOP
+// CHECK:    <TYPE_RECORD
+// CHECK:  </DECLTYPES_BLOCK>

@ChuanqiXu9 ChuanqiXu9 merged commit a4f75ec into llvm:main May 28, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category skip-precommit-approval PR for CI feedback, not intended for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants