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 10:56:44 PST 2016


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>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>
>> 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/60ad17cc/attachment.html>


More information about the cfe-commits mailing list