[PATCH][Modules][PR26179]
Vassil Vassilev via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 18 12:06:30 PST 2016
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" <vvasilev at cern.ch
> <mailto:vvasilev at cern.ch>> wrote:
>
> On 17/01/16 06:34, Douglas Gregor wrote:
>
> On Jan 16, 2016, at 3:41 PM, Vassil Vassilev
> <vvasilev at cern.ch <mailto:vvasilev at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160118/e80ec4a9/attachment.html>
-------------- next part --------------
From d90d111c53eee39c5c74d2e59b607e421c176348 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev at 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)
More information about the cfe-commits
mailing list