[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

[Brian Gesiak via Phabricator via cfe-commits]

modocache updated this revision to Diff 227808.
modocache added a comment.

Rebasing onto the monorepo. @rsmith, I confirmed the test cases that this diff 
adds still fail on trunk, and that the Clang source changes made in this diff 
fix the test case failures. Tomorrow I plan on poking around to see if I can 
reproduce similar issues in the C++20 modules implementation. But in the 
meantime, how do you feel about this patch? You suggested a change to 
`~FindExistingResult` but also that it'd be difficult to test/verify such a 
change. Is that change still something you're looking for before accepting this 
diff?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58920/new/

https://reviews.llvm.org/D58920

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/pr39287-2/a.h
  clang/test/Modules/Inputs/pr39287-2/module.modulemap
  clang/test/Modules/Inputs/pr39287/a.h
  clang/test/Modules/Inputs/pr39287/module.modulemap
  clang/test/Modules/pr39287-2.cpp
  clang/test/Modules/pr39287.cpp


Index: clang/test/Modules/pr39287.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/pr39287.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++17 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/pr39287 %s -verify
+
+class A {
+  virtual ~A() {}
+};
+
+#include "a.h"
+
+namespace std { class type_info; }
+
+void foo() {
+  typeid(foo); // expected-warning {{expression result unused}}
+}
Index: clang/test/Modules/pr39287-2.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/pr39287-2.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++17 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/pr39287-2 %s -verify
+
+class A {
+  virtual ~A() {}
+};
+
+#include "a.h"
+
+void foo() {
+  typeid(foo); // expected-warning {{expression result unused}}
+}
Index: clang/test/Modules/Inputs/pr39287/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/pr39287/module.modulemap
@@ -0,0 +1,3 @@
+module "a.h" {
+  header "a.h"
+}
Index: clang/test/Modules/Inputs/pr39287/a.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/pr39287/a.h
@@ -0,0 +1 @@
+namespace std {}
Index: clang/test/Modules/Inputs/pr39287-2/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/pr39287-2/module.modulemap
@@ -0,0 +1,3 @@
+module "a.h" {
+  header "a.h"
+}
Index: clang/test/Modules/Inputs/pr39287-2/a.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/pr39287-2/a.h
@@ -0,0 +1 @@
+namespace std { class type_info; }
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -47,6 +47,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Sema/IdentifierResolver.h"
+#include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ContinuousRangeMap.h"
@@ -3195,6 +3196,9 @@
     // Add the declaration to its redeclaration context so later merging
     // lookups will find it.
     MergeDC->makeDeclVisibleInContextImpl(New, /*Internal*/true);
+    if (isa<NamespaceDecl>(New) && Name.getAsString() == "std")
+      if (!Reader.getSema()->StdNamespace)
+        Reader.getSema()->StdNamespace = New;
   }
 }
 
@@ -3358,6 +3362,14 @@
           return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
                                     TypedefNameForLinkage);
     }
+    if (isa<NamespaceDecl>(D) && D->getName() == "std") {
+      auto StdPtr = Reader.getSema()->StdNamespace;
+      if (StdPtr.isValid() && !StdPtr.isOffset())
+        if (auto *Std = cast_or_null<NamespaceDecl>(StdPtr.get(nullptr)))
+          if (isSameEntity(Std, D))
+            return FindExistingResult(Reader, D, Std, AnonymousDeclNumber,
+                                      TypedefNameForLinkage);
+    }
   } else {
     // Not in a mergeable context.
     return FindExistingResult(Reader);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits