Attaching v3 of the patch. Added your case to the current test and fixed
my silly non-array mistake.
-- Vassil
On 18/01/16 20:38, Richard Smith wrote:
Please also add a test case that your old patch would have failed on,
such as:
m1.h:
extern int a[];
m2.h:
extern int a[5];
x.cc:
#include "m1.h"
#include "m2.h"
int *p = a;
On Jan 18, 2016 9:28 AM, "Vassil Vassilev" <vvasi...@cern.ch
<mailto:vvasi...@cern.ch>> wrote:
On 17/01/16 06:34, Douglas Gregor wrote:
On Jan 16, 2016, at 3:41 PM, Vassil Vassilev
<vvasi...@cern.ch <mailto:vvasi...@cern.ch>> wrote:
Hi,
Could somebody review the attached patch. It fixes
https://llvm.org/bugs/show_bug.cgi?id=26179
Many thanks!
Vassil
<0001-modules-Teach-clang-to-how-to-merge-variable-redecls.patch>
+ // We can get decls with different types on the redecl
chain. Eg.
+ // template <typename T> struct S { static T Var[]; };
// #1
+ // template <typename T> T S<T>::Var[sizeof(T)]; // #2
+ // Trying to compare #1 and #2 should go through their
canonical decls.
+ QualType VarXTy = VarX->getCanonicalDecl()->getType();
+ QualType VarYTy = VarY->getCanonicalDecl()->getType();
+ if (Context.hasSameType(VarXTy, VarYTy))
+ return true;
Completing an incomplete array is (I think) the only case in
which this can happen. How about checking for that case
specifically (i.e., it’s okay to have one be an incomplete
array and the other to be any other kind of array with the
same element type), rather than a blanket check on the
canonical declaration types?
- Doug
Thanks for the comments. Patch v2 attached.
-- Vassil
From d90d111c53eee39c5c74d2e59b607e421c176348 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassi...@gmail.com>
Date: Sat, 16 Jan 2016 23:52:40 +0100
Subject: [PATCH] [modules] Teach clang to how to merge variables with
incomplete array types.
We can get decls with different types on the redecl chain. Eg.
template <typename T> struct S {
static T Var[]; // #1
};
template <typename T> T S<T>::Var[sizeof(T)]; // #2
Trying to compare #1 and #2 should go through their canonical decls.
Fixes https://llvm.org/bugs/show_bug.cgi?id=26179
---
lib/Serialization/ASTReaderDecl.cpp | 20 ++++++++++++++++++--
test/Modules/Inputs/PR26179/A.h | 4 ++++
test/Modules/Inputs/PR26179/B.h | 2 ++
test/Modules/Inputs/PR26179/basic_string.h | 14 ++++++++++++++
test/Modules/Inputs/PR26179/module.modulemap | 9 +++++++++
test/Modules/pr26179.cpp | 7 +++++++
6 files changed, 54 insertions(+), 2 deletions(-)
create mode 100644 test/Modules/Inputs/PR26179/A.h
create mode 100644 test/Modules/Inputs/PR26179/B.h
create mode 100644 test/Modules/Inputs/PR26179/basic_string.h
create mode 100644 test/Modules/Inputs/PR26179/module.modulemap
create mode 100644 test/Modules/pr26179.cpp
diff --git a/lib/Serialization/ASTReaderDecl.cpp
b/lib/Serialization/ASTReaderDecl.cpp
index 5bf95f8..9bcd9a0 100644
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -2595,8 +2595,24 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) {
// Variables with the same type and linkage match.
if (VarDecl *VarX = dyn_cast<VarDecl>(X)) {
VarDecl *VarY = cast<VarDecl>(Y);
- return (VarX->getLinkageInternal() == VarY->getLinkageInternal()) &&
- VarX->getASTContext().hasSameType(VarX->getType(), VarY->getType());
+ if (VarX->getLinkageInternal() == VarY->getLinkageInternal()) {
+ ASTContext &C = VarX->getASTContext();
+ if (C.hasSameType(VarX->getType(), VarY->getType()))
+ return true;
+
+ // We can get decls with different types on the redecl chain. Eg.
+ // template <typename T> struct S { static T Var[]; }; // #1
+ // template <typename T> T S<T>::Var[sizeof(T)]; // #2
+ // Only? happens when completing an incomplete array type. In this case
+ // when comparing #1 and #2 we should go through their elements types.
+ const ArrayType *VarXTy = C.getAsArrayType(VarX->getType());
+ const ArrayType *VarYTy = C.getAsArrayType(VarY->getType());
+ if (!VarXTy || !VarYTy)
+ return false;
+ if (VarXTy->isIncompleteArrayType() || VarYTy->isIncompleteArrayType())
+ return C.hasSameType(VarXTy->getElementType(),
VarYTy->getElementType());
+ }
+ return false;
}
// Namespaces with the same name and inlinedness match.
diff --git a/test/Modules/Inputs/PR26179/A.h b/test/Modules/Inputs/PR26179/A.h
new file mode 100644
index 0000000..ce95faf
--- /dev/null
+++ b/test/Modules/Inputs/PR26179/A.h
@@ -0,0 +1,4 @@
+#include "basic_string.h"
+#include "B.h"
+
+int *p = a;
diff --git a/test/Modules/Inputs/PR26179/B.h b/test/Modules/Inputs/PR26179/B.h
new file mode 100644
index 0000000..eb8d1c2
--- /dev/null
+++ b/test/Modules/Inputs/PR26179/B.h
@@ -0,0 +1,2 @@
+#include "basic_string.h"
+extern int a[5];
diff --git a/test/Modules/Inputs/PR26179/basic_string.h
b/test/Modules/Inputs/PR26179/basic_string.h
new file mode 100644
index 0000000..afd1e0d
--- /dev/null
+++ b/test/Modules/Inputs/PR26179/basic_string.h
@@ -0,0 +1,14 @@
+#ifndef _GLIBCXX_STRING
+#define _GLIBCXX_STRING 1
+
+template<typename T>
+struct basic_string {
+ static T _S_empty_rep_storage[];
+};
+
+template<typename T>
+T basic_string<T>::_S_empty_rep_storage[sizeof(T)];
+
+extern int a[];
+
+#endif
diff --git a/test/Modules/Inputs/PR26179/module.modulemap
b/test/Modules/Inputs/PR26179/module.modulemap
new file mode 100644
index 0000000..4937418
--- /dev/null
+++ b/test/Modules/Inputs/PR26179/module.modulemap
@@ -0,0 +1,9 @@
+module A {
+ header "A.h"
+ export *
+}
+
+module B {
+ header "B.h"
+ export *
+}
diff --git a/test/Modules/pr26179.cpp b/test/Modules/pr26179.cpp
new file mode 100644
index 0000000..f25f1ce
--- /dev/null
+++ b/test/Modules/pr26179.cpp
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -I%S/Inputs/PR26179 -verify %s
+// RUN: %clang_cc1 -fmodules
-fmodule-map-file=%S/Inputs/PR26179/module.modulemap -fmodules-cache-path=%t
-I%S/Inputs/PR26179 -verify %s
+
+#include "A.h"
+
+// expected-no-diagnostics
--
2.3.8 (Apple Git-58)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
