Revert "[Modules] Remove unnecessary check when generating name lookup table in ASTWr...

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.

diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index d31fa38..09ee174 100644 (file)
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -514,6 +514,7 @@ private:
   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 5f8f5d3..c1afdeb 100644 (file)
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3849,6 +3849,12 @@ public:
 
 } // 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 (file)
index 44fa367..0000000
--- 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 _;
-}