On Fri, Feb 5, 2016 at 7:04 AM, Vassil Vassilev <v.g.vassi...@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@lists.llvm.org>cfe-commits@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > > > _______________________________________________ > cfe-commits mailing > listcfe-comm...@lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
