r258524 - Merge templated static member variables, fixes http://llvm.org/pr26179.
Vassil Vassilev via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 5 12:27:23 PST 2016
I am not sure where else to put this logic if not in isSameEntity...
Could you point me to a better place?
--Vassil
On 05/02/16 19:56, Richard Smith wrote:
> On Fri, Feb 5, 2016 at 7:04 AM, Vassil Vassilev
> <v.g.vassilev at gmail.com <mailto:v.g.vassilev at gmail.com>> wrote:
>
> Good point. Do you have in mind calling
> 'clang::Sema::MergeVarDeclTypes' or we to just "duplicate" the
> logic in clang::ASTDeclReader::mergeRedeclarable?
>
>
> It's not safe to call into Sema while we're in a
> partially-deserialized state. We know the only interesting case here
> is complete versus incomplete array types, so we don't need anything
> like the complexity of MergeVarDeclTypes -- something as easy as "if
> (new type is incomplete and old type is not) set new type to old type"
> should work.
>
> On 05/02/16 02:50, Richard Smith via cfe-commits wrote:
>> I suspect we'll need to do a little more than this: when we
>> rebuild the redeclaration chain, we should probably propagate the
>> complete type to later redeclarations in the same scope.
>> Otherwise, if we import a module that has a complete type and one
>> that has an incomplete type, the declaration found by name lookup
>> might be the one with the incomplete type, possibly resulting in
>> rejects-valid on code like this:
>>
>> a.h:
>> extern int a[5];
>>
>> b.h:
>> extern int a[];
>>
>> x.cc:
>> #include "a.h"
>> #include "b.h"
>> int k = sizeof(a);
>>
>> On Fri, Jan 22, 2016 at 11:03 AM, Yaron Keren via cfe-commits
>> <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>
>> wrote:
>>
>> Author: yrnkrn
>> Date: Fri Jan 22 13:03:27 2016
>> New Revision: 258524
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=258524&view=rev
>> Log:
>> Merge templated static member variables, fixes
>> http://llvm.org/pr26179.
>>
>> Patch by Vassil Vassilev!
>> Reviewed by Richard Smith.
>>
>>
>> Added:
>> cfe/trunk/test/Modules/Inputs/PR26179/
>> cfe/trunk/test/Modules/Inputs/PR26179/A.h
>> cfe/trunk/test/Modules/Inputs/PR26179/B.h
>> cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h
>> cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
>> cfe/trunk/test/Modules/pr26179.cpp
>> Modified:
>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>>
>> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=258524&r1=258523&r2=258524&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
>> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Jan 22
>> 13:03:27 2016
>> @@ -2595,8 +2595,24 @@ static bool isSameEntity(NamedDecl *X, N
>> // 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.
>>
>> Added: cfe/trunk/test/Modules/Inputs/PR26179/A.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/A.h?rev=258524&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/PR26179/A.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/PR26179/A.h Fri Jan 22
>> 13:03:27 2016
>> @@ -0,0 +1,4 @@
>> +#include "basic_string.h"
>> +#include "B.h"
>> +
>> +int *p = a;
>>
>> Added: cfe/trunk/test/Modules/Inputs/PR26179/B.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/B.h?rev=258524&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/PR26179/B.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/PR26179/B.h Fri Jan 22
>> 13:03:27 2016
>> @@ -0,0 +1,2 @@
>> +#include "basic_string.h"
>> +extern int a[5];
>>
>> Added: cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h?rev=258524&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h Fri
>> Jan 22 13:03:27 2016
>> @@ -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
>>
>> Added: cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap?rev=258524&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
>> (added)
>> +++ cfe/trunk/test/Modules/Inputs/PR26179/module.modulemap
>> Fri Jan 22 13:03:27 2016
>> @@ -0,0 +1,9 @@
>> +module A {
>> + header "A.h"
>> + export *
>> +}
>> +
>> +module B {
>> + header "B.h"
>> + export *
>> +}
>>
>> Added: cfe/trunk/test/Modules/pr26179.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr26179.cpp?rev=258524&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/Modules/pr26179.cpp (added)
>> +++ cfe/trunk/test/Modules/pr26179.cpp Fri Jan 22 13:03:27 2016
>> @@ -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
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160205/a8f9e0c1/attachment-0001.html>
-------------- next part --------------
From 22f71c24305c204b8beedc3e69c5cb1ed1ccf1a0 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev at gmail.com>
Date: Fri, 5 Feb 2016 21:15:05 +0100
Subject: [PATCH] [modules] The redecl chain needs to be updated if the
complete array type is known.
---
lib/Serialization/ASTReaderDecl.cpp | 11 +++++++++--
test/Modules/Inputs/PR26179/A.h | 2 --
test/Modules/Inputs/PR26179/B.h | 1 -
test/Modules/Inputs/PR26179/basic_string.h | 2 --
test/Modules/Inputs/merge-incomplete-array-vars/a.h | 4 ++++
test/Modules/Inputs/merge-incomplete-array-vars/b.h | 4 ++++
.../Inputs/merge-incomplete-array-vars/module.modulemap | 2 ++
test/Modules/merge-incomplete-array-vars.cpp | 17 +++++++++++++++++
8 files changed, 36 insertions(+), 7 deletions(-)
create mode 100644 test/Modules/Inputs/merge-incomplete-array-vars/a.h
create mode 100644 test/Modules/Inputs/merge-incomplete-array-vars/b.h
create mode 100644 test/Modules/Inputs/merge-incomplete-array-vars/module.modulemap
create mode 100644 test/Modules/merge-incomplete-array-vars.cpp
diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 9bcd9a0..b7e165c 100644
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -2604,13 +2604,20 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) {
// 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.
+ // when comparing #1 and #2 we should go through their element type.
const ArrayType *VarXTy = C.getAsArrayType(VarX->getType());
const ArrayType *VarYTy = C.getAsArrayType(VarY->getType());
if (!VarXTy || !VarYTy)
return false;
- if (VarXTy->isIncompleteArrayType() || VarYTy->isIncompleteArrayType())
+ if (VarXTy->isIncompleteArrayType() || VarYTy->isIncompleteArrayType()) {
+ // We want to make sure that our redecl chain is consistent, containing
+ // the most complete type. See Sema::MergeVarDeclTypes.
+ if (VarXTy->isIncompleteArrayType() && VarYTy->isArrayType())
+ VarX->setType(VarY->getType());
+ else if (VarXTy->isArrayType() && VarYTy->isIncompleteArrayType())
+ VarY->setType(VarX->getType());
return C.hasSameType(VarXTy->getElementType(), VarYTy->getElementType());
+ }
}
return false;
}
diff --git a/test/Modules/Inputs/PR26179/A.h b/test/Modules/Inputs/PR26179/A.h
index ce95faf..c264f4c 100644
--- a/test/Modules/Inputs/PR26179/A.h
+++ b/test/Modules/Inputs/PR26179/A.h
@@ -1,4 +1,2 @@
#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
index eb8d1c2..46a109e 100644
--- a/test/Modules/Inputs/PR26179/B.h
+++ b/test/Modules/Inputs/PR26179/B.h
@@ -1,2 +1 @@
#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
index afd1e0d..653ce07 100644
--- a/test/Modules/Inputs/PR26179/basic_string.h
+++ b/test/Modules/Inputs/PR26179/basic_string.h
@@ -9,6 +9,4 @@ struct basic_string {
template<typename T>
T basic_string<T>::_S_empty_rep_storage[sizeof(T)];
-extern int a[];
-
#endif
diff --git a/test/Modules/Inputs/merge-incomplete-array-vars/a.h b/test/Modules/Inputs/merge-incomplete-array-vars/a.h
new file mode 100644
index 0000000..4dcf7de
--- /dev/null
+++ b/test/Modules/Inputs/merge-incomplete-array-vars/a.h
@@ -0,0 +1,4 @@
+extern int a[5];
+extern int aa[55];
+extern int b[];
+extern int bb[];
diff --git a/test/Modules/Inputs/merge-incomplete-array-vars/b.h b/test/Modules/Inputs/merge-incomplete-array-vars/b.h
new file mode 100644
index 0000000..8486136
--- /dev/null
+++ b/test/Modules/Inputs/merge-incomplete-array-vars/b.h
@@ -0,0 +1,4 @@
+extern int a[];
+extern int aa[];
+extern int b[6];
+extern int bb[65];
diff --git a/test/Modules/Inputs/merge-incomplete-array-vars/module.modulemap b/test/Modules/Inputs/merge-incomplete-array-vars/module.modulemap
new file mode 100644
index 0000000..2da1c03
--- /dev/null
+++ b/test/Modules/Inputs/merge-incomplete-array-vars/module.modulemap
@@ -0,0 +1,2 @@
+module A { header "a.h" export * }
+module B { header "b.h" export * }
diff --git a/test/Modules/merge-incomplete-array-vars.cpp b/test/Modules/merge-incomplete-array-vars.cpp
new file mode 100644
index 0000000..2970a90
--- /dev/null
+++ b/test/Modules/merge-incomplete-array-vars.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -I%S/Inputs/merge-incomplete-array-vars -verify %s
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/merge-incomplete-array-vars -verify %s
+// expected-no-diagnostics
+
+extern int aa[];
+extern int bb[];
+
+#include "a.h"
+#include "b.h"
+
+// Import the complete type first and the incomplete second, forcing the
+// most recent decl to have an incomplete type.
+int k = sizeof(a);
+int l = sizeof(aa);
+int m = sizeof(b);
+int n = sizeof(bb);
--
2.3.8 (Apple Git-58)
More information about the cfe-commits
mailing list