r258524 - Merge templated static member variables, fixes http://llvm.org/pr26179.
Vassil Vassilev via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 18 07:56:49 PST 2016
Š¢hanks, could you (or smb else with commit perms) check it in?
--Vassil
On 18/02/16 03:24, Richard Smith wrote:
> (And otherwise this LGTM.)
>
> On Wed, Feb 17, 2016 at 5:24 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> + // TODO: Check visibility. New is hidden but has a complete type. If New
>> + // has no array bound, it should not inherit one from Old, if Old is not
>> + // visible.
>>
>> I think this is in the wrong place: it should be attached to the "old
>> is complete and new is not" case. Also, our convention is to use
>> FIXME, not TODO.
>>
>> Speaking of which, you can now delete the "FIXME: Even if this merging
>> succeeds, [...]" comment now, and add its example as a testcase. With
>> your change we should reliably diagnose it.
>>
>> On Wed, Feb 17, 2016 at 2:32 PM, Vassil Vassilev <v.g.vassilev at gmail.com> wrote:
>>> On 16/02/16 22:20, Richard Smith wrote:
>>>
>>> --- a/lib/Sema/SemaLookup.cpp
>>> +++ b/lib/Sema/SemaLookup.cpp
>>> @@ -419,6 +419,25 @@ static bool isPreferredLookupResult(Sema &S,
>>> Sema::LookupNameKind Kind,
>>> }
>>> }
>>>
>>> + // VarDecl can have incomplete array types, prefer the one with more
>>> complete
>>> + // array type.
>>> + if (VarDecl *DVD = dyn_cast<VarDecl>(DUnderlying)) {
>>> + VarDecl *EVD = cast<VarDecl>(EUnderlying);
>>> + // Skip local VarDecls with incomplete array type.
>>> + if ((!DVD->isLocalVarDecl() && DVD->hasExternalStorage()) ||
>>> + (!EVD->isLocalVarDecl() && EVD->hasExternalStorage())) {
>>>
>>> Why do you need this special case?
>>>
>>> No reason, I decided to be conservative. All comments should be addressed in
>>> the new version of the patch.
>>>
>>> +
>>> + ASTContext &C = S.getASTContext();
>>> +
>>> + if (const ArrayType *DVDTy = C.getAsArrayType(DVD->getType())) {
>>> + const ArrayType *EVDTy = C.getAsArrayType(EVD->getType());
>>> + // Prefer the decl with more complete type if visible.
>>> + return !DVDTy->isIncompleteArrayType() &&
>>> EVDTy->isIncompleteArrayType()
>>>
>>> Checking for an array type here seems unnecessary -- it'd be simpler to
>>> check EVD->getType()->isIncompleteType() &&
>>> !DVD->getType()->isIncompleteType().
>>>
>>> + && S.isVisible(D);
>>>
>>> This seems like a very subtle case: we're performing redeclaration lookup
>>> for a variable declaration X, and we find two results D and E. D is hidden
>>> but has a complete type. E is visible but has an incomplete type. If X has
>>> no array bound, it should not inherit one from D. But if it does have an
>>> array bound, and that bound differs from E's bound, we should diagnose the
>>> mismatch.
>>>
>>> Please add another test to the end of merge-incomplete-array-vars.cpp to
>>> check this is working:
>>> int c[2]; // expected-error {{different type: 'int [2]' vs 'int [1]'}}
>>>
>>> That said, I think this test will fail if you reorder c1 and c2 in the
>>> module map, but that's a pre-existing bug (see the last FIXME in
>>> Sema::MergeVarDeclTypes).
>>>
>>> + }
>>> + }
>>> + }
>>>
>>>
>>> On Tue, Feb 16, 2016 at 11:12 AM, Vassil Vassilev <v.g.vassilev at gmail.com>
>>> wrote:
>>>> ping...
>>>>
>>>> On 07/02/16 22:33, Vassil Vassilev wrote:
>>>>
>>>> Improve a comment.
>>>> --Vassil
>>>> On 07/02/16 20:48, Vassil Vassilev wrote:
>>>>
>>>> Would this patch be any better?
>>>> --Vassil
>>>> On 05/02/16 21:49, Richard Smith wrote:
>>>>
>>>> This belongs in ASTDeclReader::attachPreviousDecl[Impl]. That's where we
>>>> propagate data that's supposed to be consistent across a redeclaration chain
>>>> from earlier declarations to later ones.
>>>>
>>>> There's another wrinkle here: we should only be propagating the type from
>>>> a previous declaration that's declared in the same scope (in particular, we
>>>> should skip over declarations declared as local extern declarations within a
>>>> function). This may in some cases require walking backwards along the
>>>> redeclaration chain to find the previous declaration that was not a local
>>>> extern declaration. That'd be observable in a case like this:
>>>>
>>>> modulemap:
>>>> module A { module A1 { header "a1.h" } module A2 { header "a2.h" } }
>>>> module B { header "b.h" }
>>>>
>>>> a1.h:
>>>> int a[];
>>>>
>>>> b.h:
>>>> void g(int(*)[], int);
>>>> void g(int(*)[1], int) = delete;
>>>> template<typename T> void f(T t) {
>>>> extern int a[];
>>>> g(&a, t);
>>>> }
>>>>
>>>> a2.h:
>>>> int a[1];
>>>>
>>>> x.cc:
>>>> #include "a1.h"
>>>> #include "b.h"
>>>> void h() { f(0); } // should not produce an error; type of 'a' within 'f'
>>>> should not have been updated
>>>>
>>>> That example actually reveals another problem: we really don't want the
>>>> completed type to be visible unless there is a visible declaration with the
>>>> completed type. That suggests that propagating the type across the
>>>> redeclaration chain may be the wrong approach, and we should instead handle
>>>> this by changing isPreferredLookupResult (in SemaLookup.cpp) to prefer a
>>>> VarDecl with a complete type over one with an incomplete type.
>>>>
>>>> On Fri, Feb 5, 2016 at 12:27 PM, Vassil Vassilev <v.g.vassilev at gmail.com>
>>>> wrote:
>>>>> 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>
>>>>> 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> 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
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
-------------- next part --------------
From 61d99ea390f286c2d77e03b13cda114704f1fb70 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] Prefer more complete variable types.
---
lib/Sema/SemaDecl.cpp | 83 +++++++++++++---------
lib/Sema/SemaLookup.cpp | 12 ++++
lib/Serialization/ASTReaderDecl.cpp | 2 +-
test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp | 4 ++
test/Modules/Inputs/PR26179/A.h | 2 -
test/Modules/Inputs/PR26179/B.h | 1 -
test/Modules/Inputs/PR26179/basic_string.h | 2 -
.../Modules/Inputs/merge-incomplete-array-vars/a.h | 4 ++
.../Modules/Inputs/merge-incomplete-array-vars/b.h | 4 ++
.../Inputs/merge-incomplete-array-vars/c1.h | 1 +
.../Inputs/merge-incomplete-array-vars/c2.h | 1 +
.../Modules/Inputs/merge-incomplete-array-vars/d.h | 7 ++
.../merge-incomplete-array-vars/module.modulemap | 5 ++
test/Modules/merge-incomplete-array-vars.cpp | 21 ++++++
14 files changed, 109 insertions(+), 40 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/c1.h
create mode 100644 test/Modules/Inputs/merge-incomplete-array-vars/c2.h
create mode 100644 test/Modules/Inputs/merge-incomplete-array-vars/d.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/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 66c0e05..8b6acbf 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -3245,6 +3245,22 @@ void Sema::mergeObjCMethodDecls(ObjCMethodDecl *newMethod,
CheckObjCMethodOverride(newMethod, oldMethod);
}
+static void diagnoseVarDeclTypeMismatch(Sema &S, VarDecl *New, VarDecl* Old) {
+ assert(!S.Context.hasSameType(New->getType(), Old->getType()));
+
+ S.Diag(New->getLocation(), New->isThisDeclarationADefinition()
+ ? diag::err_redefinition_different_type
+ : diag::err_redeclaration_different_type)
+ << New->getDeclName() << New->getType() << Old->getType();
+
+ diag::kind PrevDiag;
+ SourceLocation OldLocation;
+ std::tie(PrevDiag, OldLocation)
+ = getNoteDiagForInvalidRedeclaration(Old, New);
+ S.Diag(OldLocation, PrevDiag);
+ New->setInvalidDecl();
+}
+
/// MergeVarDeclTypes - We parsed a variable 'New' which has the same name and
/// scope as a previous declaration 'Old'. Figure out how to merge their types,
/// emitting diagnostics as appropriate.
@@ -3271,21 +3287,40 @@ void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old,
// object or function shall be identical, except that declarations for an
// array object can specify array types that differ by the presence or
// absence of a major array bound (8.3.4).
- else if (Old->getType()->isIncompleteArrayType() &&
- New->getType()->isArrayType()) {
+ else if (Old->getType()->isArrayType() && New->getType()->isArrayType()) {
const ArrayType *OldArray = Context.getAsArrayType(Old->getType());
const ArrayType *NewArray = Context.getAsArrayType(New->getType());
- if (Context.hasSameType(OldArray->getElementType(),
- NewArray->getElementType()))
- MergedT = New->getType();
- } else if (Old->getType()->isArrayType() &&
- New->getType()->isIncompleteArrayType()) {
- const ArrayType *OldArray = Context.getAsArrayType(Old->getType());
- const ArrayType *NewArray = Context.getAsArrayType(New->getType());
- if (Context.hasSameType(OldArray->getElementType(),
- NewArray->getElementType()))
- MergedT = Old->getType();
- } else if (New->getType()->isObjCObjectPointerType() &&
+
+ // We are merging a variable declaration New into Old. If it has an array
+ // bound, and that bound differs from Old's bound, we should diagnose the
+ // mismatch.
+ if (!NewArray->isIncompleteArrayType()) {
+ for (VarDecl *PrevVD = Old->getMostRecentDecl(); PrevVD;
+ PrevVD = PrevVD->getPreviousDecl()) {
+ const ArrayType *PrevVDTy = Context.getAsArrayType(PrevVD->getType());
+ if (PrevVDTy->isIncompleteArrayType())
+ continue;
+
+ if (!Context.hasSameType(NewArray, PrevVDTy))
+ return diagnoseVarDeclTypeMismatch(*this, New, PrevVD);
+ }
+ }
+
+ if (OldArray->isIncompleteArrayType() && NewArray->isArrayType()) {
+ if (Context.hasSameType(OldArray->getElementType(),
+ NewArray->getElementType()))
+ MergedT = New->getType();
+ }
+ // FIXME: Check visibility. New is hidden but has a complete type. If New
+ // has no array bound, it should not inherit one from Old, if Old is not
+ // visible.
+ else if (OldArray->isArrayType() && NewArray->isIncompleteArrayType()) {
+ if (Context.hasSameType(OldArray->getElementType(),
+ NewArray->getElementType()))
+ MergedT = Old->getType();
+ }
+ }
+ else if (New->getType()->isObjCObjectPointerType() &&
Old->getType()->isObjCObjectPointerType()) {
MergedT = Context.mergeObjCGCQualifiers(New->getType(),
Old->getType());
@@ -3311,27 +3346,7 @@ void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old,
New->setType(Context.DependentTy);
return;
}
-
- // FIXME: Even if this merging succeeds, some other non-visible declaration
- // of this variable might have an incompatible type. For instance:
- //
- // extern int arr[];
- // void f() { extern int arr[2]; }
- // void g() { extern int arr[3]; }
- //
- // Neither C nor C++ requires a diagnostic for this, but we should still try
- // to diagnose it.
- Diag(New->getLocation(), New->isThisDeclarationADefinition()
- ? diag::err_redefinition_different_type
- : diag::err_redeclaration_different_type)
- << New->getDeclName() << New->getType() << Old->getType();
-
- diag::kind PrevDiag;
- SourceLocation OldLocation;
- std::tie(PrevDiag, OldLocation) =
- getNoteDiagForInvalidRedeclaration(Old, New);
- Diag(OldLocation, PrevDiag);
- return New->setInvalidDecl();
+ return diagnoseVarDeclTypeMismatch(*this, New, Old);
}
// Don't actually update the type on the new declaration if the old
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 1d7b15c..7405032 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -419,6 +419,18 @@ static bool isPreferredLookupResult(Sema &S, Sema::LookupNameKind Kind,
}
}
+ // VarDecl can have incomplete array types, prefer the one with more complete
+ // array type.
+ if (VarDecl *DVD = dyn_cast<VarDecl>(DUnderlying)) {
+ VarDecl *EVD = cast<VarDecl>(EUnderlying);
+ if (EVD->getType()->isIncompleteType() &&
+ !DVD->getType()->isIncompleteType()) {
+ // Prefer the decl with a more complete type if visible.
+ return S.isVisible(DVD);
+ }
+ return false; // Avoid picking up a newer decl, just because it was newer.
+ }
+
// For most kinds of declaration, it doesn't really matter which one we pick.
if (!isa<FunctionDecl>(DUnderlying) && !isa<VarDecl>(DUnderlying)) {
// If the existing declaration is hidden, prefer the new one. Otherwise,
diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 9bcd9a0..feb9d12 100644
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -2604,7 +2604,7 @@ 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)
diff --git a/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp b/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
index 4686b1c..188a0a2 100644
--- a/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
+++ b/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
@@ -207,3 +207,7 @@ namespace use_outside_ns {
int j() { return sizeof(d); }
}
}
+
+extern int arr[];
+void f1() { extern int arr[2]; } // expected-note {{previous}}
+void f2() { extern int arr[3]; } // expected-error {{different type: 'int [3]' vs 'int [2]'}}
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/c1.h b/test/Modules/Inputs/merge-incomplete-array-vars/c1.h
new file mode 100644
index 0000000..99be230
--- /dev/null
+++ b/test/Modules/Inputs/merge-incomplete-array-vars/c1.h
@@ -0,0 +1 @@
+extern int c[];
diff --git a/test/Modules/Inputs/merge-incomplete-array-vars/c2.h b/test/Modules/Inputs/merge-incomplete-array-vars/c2.h
new file mode 100644
index 0000000..3fee8c7
--- /dev/null
+++ b/test/Modules/Inputs/merge-incomplete-array-vars/c2.h
@@ -0,0 +1 @@
+extern int c[1];
diff --git a/test/Modules/Inputs/merge-incomplete-array-vars/d.h b/test/Modules/Inputs/merge-incomplete-array-vars/d.h
new file mode 100644
index 0000000..e24cf76
--- /dev/null
+++ b/test/Modules/Inputs/merge-incomplete-array-vars/d.h
@@ -0,0 +1,7 @@
+extern int c[];
+void g(int(*)[], int);
+void g(int(*)[1], int) = delete;
+template<typename T> void f(T t) {
+ extern int c[];
+ g(&c, t);
+}
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..d2b467d
--- /dev/null
+++ b/test/Modules/Inputs/merge-incomplete-array-vars/module.modulemap
@@ -0,0 +1,5 @@
+module A { header "a.h" export * }
+module B { header "b.h" export * }
+
+module C { module C1 { header "c1.h" } module C2 { header "c2.h" } }
+module D { header "d.h" }
diff --git a/test/Modules/merge-incomplete-array-vars.cpp b/test/Modules/merge-incomplete-array-vars.cpp
new file mode 100644
index 0000000..dd08b64
--- /dev/null
+++ b/test/Modules/merge-incomplete-array-vars.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/merge-incomplete-array-vars -verify %s
+
+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);
+
+#include "c1.h"
+#include "d.h"
+int size = sizeof(c); // expected-error{{invalid application of 'sizeof' to an incomplete type 'int []'}}
+void h() { f(0); } // should not produce an error; type of 'c' within 'f' should not have been updated
+int c[2]; // expected-error {{different type: 'int [2]' vs 'int [1]'}} //expected-note at c2.h:1 {{previous declaration is here}}
--
2.3.8 (Apple Git-58)
More information about the cfe-commits
mailing list