r258524 - Merge templated static member variables, fixes http://llvm.org/pr26179.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 5 12:49:05 PST 2016
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>
> 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>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 listcfe-commits at lists.llvm.orghttp://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/838ae9bc/attachment-0001.html>
More information about the cfe-commits
mailing list