[clang] 1e07091 - Revert "[Modules] Remove unnecessary check when generating name lookup table in ASTWriter"

[Krasimir Georgiev via cfe-commits]

Author: Krasimir Georgiev
Date: 2023-03-10T14:14:55+01:00
New Revision: 1e0709167f5edd330889f51bb203c458bdb5e359

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

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

This reverts commit db987b9589be1eb604fcb74c85b410469e31485f.

We're seeing failures in modules-enabled builds from within stdlib after
this commit. Errors look like:

In module '...stl_cc_library':
...optional:560:38: error: 'std::__optional_copy_assign_base<unsigned
long>::__optional_copy_assign_base' from module '...optional' is not
present in definition of 'std::__optional_copy_assign_base<unsigned
long>' in module '...optional'
    using __optional_move_base<_Tp>::__optional_move_base;

In module '...stl_cc_library':
...optional:771:11: error: no matching constructor for initialization of '__optional_move_assign_base<unsigned long>'
        : __base(in_place, _VSTD::forward<_Up>(__v)) {}
          ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
llvm-project/clang/include/clang/Basic/CodeGenOptions.h:448:57: note: in instantiation of function template specialization 'std::optional<unsigned long>::optional<int, 0>' requested here
  std::optional<uint64_t> DiagnosticsHotnessThreshold = 0;

I don't have a self-contained reproducer at this point. I'm hoping that
we may be able to share more information about these issues later, if
necessary.

Added: 
    

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

Removed: 
    clang/test/Modules/pr61065.cppm


################################################################################
diff  --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index d31fa38b93825..09ee1744e8945 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -514,6 +514,7 @@ 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 5f8f5d38932a5..c1afdeb6007db 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3849,6 +3849,12 @@ 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())
@@ -3891,7 +3897,8 @@ 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 (isLookupResultEntirelyExternal(Result, DC))
+    if (isLookupResultExternal(Result, DC) &&
+        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
deleted file mode 100644
index 44fa3679974ad..0000000000000
--- a/clang/test/Modules/pr61065.cppm
+++ /dev/null
@@ -1,55 +0,0 @@
-// 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 _;
-}