modocache created this revision.
modocache added reviewers: rsmith, andrewjcg.
Herald added a project: clang.
This patch addresses https://bugs.llvm.org/show_bug.cgi?id=39287.
Clang creates a 'std' namespace in one of two ways: either it parses a
`namespace std { ... }` declaration, or it creates one implicitly.
One case in which Clang creates an implicit 'std' namespace is when it
encounters a virtual destructor in C++17, which results in the implicit
definition of operator new and delete overloads that take a
`std::align_val_t` argument (this behavior was added in rL282800
<https://reviews.llvm.org/rL282800>).
Unfortunately, when using Clang modules, further definitions of the
`std` namespace may or may not reconcile with previous definitions. For
example, consider the two test cases in this patch:
- `mod-virtual-destructor-bug` defines `std::type_info` within the main
translation unit. When it does so, there are two 'std' namespaces available:
the implicitly defined one that is returned by `Sema::getStdNamespace`, and the
explicit one defined in the `a.h` module. The `std::type_info` definition ends
up being attached to the module's `std` namespace, and so the subsequent lookup
of `getStdNamespace`'s `type_info` member fails. To fix this case, this patch
adds logic to ensure `Sema::ActOnNamespaceDef` finds the `getStdNamespace`
namespace from the current translation unit.
- `mod-virtual-destructor-bug-two` defines `std::type_info` within a module.
Later, the lookup of `std::type_info` finds the implicitly created
`getStdNamespace`, which only defines `std::align_val_t`, not `std::type_info`.
To fix this case, this patch adds logic that ensures external AST sources are
loaded when looking up `std::type_info`.
Repository:
rC Clang
https://reviews.llvm.org/D58920
Files:
lib/Sema/SemaDeclCXX.cpp
lib/Serialization/ASTReader.cpp
test/Modules/Inputs/mod-virtual-destructor-bug-two/a.h
test/Modules/Inputs/mod-virtual-destructor-bug-two/module.modulemap
test/Modules/Inputs/mod-virtual-destructor-bug/a.h
test/Modules/Inputs/mod-virtual-destructor-bug/module.modulemap
test/Modules/mod-virtual-destructor-bug-two.cpp
test/Modules/mod-virtual-destructor-bug.cpp
Index: test/Modules/mod-virtual-destructor-bug.cpp
===================================================================
--- /dev/null
+++ test/Modules/mod-virtual-destructor-bug.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/mod-virtual-destructor-bug %s -verify
+
+class A {
+ virtual ~A() {}
+};
+
+#include "a.h"
+
+namespace std { class type_info; }
+
+void foo() {
+ typeid(foo); // expected-warning {{expression result unused}}
+}
Index: test/Modules/mod-virtual-destructor-bug-two.cpp
===================================================================
--- /dev/null
+++ test/Modules/mod-virtual-destructor-bug-two.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/mod-virtual-destructor-bug-two %s -verify
+
+class A {
+ virtual ~A() {}
+};
+
+#include "a.h"
+
+void foo() {
+ typeid(foo); // expected-warning {{expression result unused}}
+}
Index: test/Modules/Inputs/mod-virtual-destructor-bug/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/mod-virtual-destructor-bug/module.modulemap
@@ -0,0 +1,3 @@
+module "a.h" {
+ header "a.h"
+}
Index: test/Modules/Inputs/mod-virtual-destructor-bug/a.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/mod-virtual-destructor-bug/a.h
@@ -0,0 +1 @@
+namespace std {}
Index: test/Modules/Inputs/mod-virtual-destructor-bug-two/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/mod-virtual-destructor-bug-two/module.modulemap
@@ -0,0 +1,3 @@
+module "a.h" {
+ header "a.h"
+}
Index: test/Modules/Inputs/mod-virtual-destructor-bug-two/a.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/mod-virtual-destructor-bug-two/a.h
@@ -0,0 +1 @@
+namespace std { class type_info; }
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -7533,8 +7533,15 @@
return false;
auto It = Lookups.find(DC);
- if (It == Lookups.end())
- return false;
+ if (It == Lookups.end()) {
+ if (getContext().getLangOpts().Modules && DC->isStdNamespace()) {
+ for (auto StdIt = Lookups.begin(); StdIt != Lookups.end(); StdIt++)
+ if (StdIt->first->isStdNamespace())
+ It = StdIt;
+ }
+ if (It == Lookups.end())
+ return false;
+ }
Deserializing LookupResults(this);
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -8912,6 +8912,11 @@
if (IsInline != PrevNS->isInline())
DiagnoseNamespaceInlineMismatch(*this, NamespaceLoc, Loc, II,
&IsInline, PrevNS);
+ if (II->isStr("std") && PrevNS->isStdNamespace() &&
+ PrevNS != getStdNamespace()) {
+ PrevNS = getStdNamespace();
+ IsStd = true;
+ }
} else if (PrevDecl) {
// This is an invalid name redefinition.
Diag(Loc, diag::err_redefinition_different_kind)
@@ -9206,6 +9211,8 @@
&PP.getIdentifierTable().get("std"),
/*PrevDecl=*/nullptr);
getStdNamespace()->setImplicit(true);
+ if (getLangOpts().Modules)
+ getStdNamespace()->setHasExternalVisibleStorage(true);
}
return getStdNamespace();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
