[clang] db987b9 - [Modules] Remove unnecessary check when generating name lookup table in ASTWriter

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-03-09T17:29:36+08:00
New Revision: db987b9589be1eb604fcb74c85b410469e31485f

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

LOG: [Modules] Remove unnecessary check when generating name lookup table in ASTWriter

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

We will avoid writing the names from external AST naturally. But
currently its check is often false positive since we may have already
marked the declarations as external but
DeclContext::hasNeedToReconcileExternalVisibleStorage would be false
after reconciling.

Tested with libcxx's modular build.

This patch can improve 8% compilation time in an internal workloads.

Added: 
    clang/test/Modules/pr61065.cppm

Modified: 
    clang/include/clang/Serialization/ASTWriter.h
    clang/lib/Serialization/ASTWriter.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index 09ee1744e8945..d31fa38b93825 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -514,7 +514,6 @@ class ASTWriter : public ASTDeserializationListener,
   void WriteTypeAbbrevs();
   void WriteType(QualType T);
 
-  bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC);
   bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext *DC);
 
   void GenerateNameLookupTable(const DeclContext *DC,

diff  --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index c1afdeb6007db..5f8f5d38932a5 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3849,12 +3849,6 @@ class ASTDeclContextNameLookupTrait {
 
 } // namespace
 
-bool ASTWriter::isLookupResultExternal(StoredDeclsList &Result,
-                                       DeclContext *DC) {
-  return Result.hasExternalDecls() &&
-         DC->hasNeedToReconcileExternalVisibleStorage();
-}
-
 bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList &Result,
                                                DeclContext *DC) {
   for (auto *D : Result.getLookupResult())
@@ -3897,8 +3891,7 @@ ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC,
     // don't need to write an entry for the name at all. If we can't
     // write out a lookup set without performing more deserialization,
     // just skip this entry.
-    if (isLookupResultExternal(Result, DC) &&
-        isLookupResultEntirelyExternal(Result, DC))
+    if (isLookupResultEntirelyExternal(Result, DC))
       continue;
 
     // We also skip empty results. If any of the results could be external and

diff  --git a/clang/test/Modules/pr61065.cppm b/clang/test/Modules/pr61065.cppm
new file mode 100644
index 0000000000000..44fa3679974ad
--- /dev/null
+++ b/clang/test/Modules/pr61065.cppm
@@ -0,0 +1,55 @@
+// From https://github.com/llvm/llvm-project/issues/61065
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -o %t/b.pcm \
+// RUN:     -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/c.cppm -emit-module-interface -o %t/c.pcm \
+// RUN:     -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/d.cpp -fsyntax-only -verify -fprebuilt-module-path=%t
+
+//--- a.cppm
+export module a;
+
+struct base {
+	base(int) {}
+};
+
+export struct a : base {
+	using base::base;
+};
+
+//--- b.cppm
+export module b;
+
+import a;
+
+a b() {
+	return a(1);
+}
+
+//--- c.cppm
+export module c;
+
+import a;
+import b;
+
+struct noncopyable {
+	noncopyable(noncopyable const &) = delete;
+    noncopyable() = default;
+};
+
+export struct c {
+	noncopyable c0;
+	a c1 = 43;
+    c() = default;
+};
+
+//--- d.cpp
+// expected-no-diagnostics
+import c;
+void d() {
+    c _;
+}