r258524 - Merge templated static member variables, fixes http://llvm.org/pr26179.
Vassil Vassilev via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 16 11:12:59 PST 2016
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 <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/20160216/0bb95cc9/attachment-0001.html>
More information about the cfe-commits
mailing list