I’ve come up with a patch and sent it to the list today since it is urgent to fix.
https://reviews.llvm.org/D24110 <https://reviews.llvm.org/D24110> Thank you. > On Aug 31, 2016, at 1:50 PM, Vassil Vassilev <v.g.vassi...@gmail.com> wrote: > > Hi Akira, > Thanks for your email. I am on vacation right now and I will try to look at > this issue as soon as I can. > Sorry for the inconvenience. > Cheers, > Vassil > On 31/08/16 22:56, Akira Hatanaka wrote: >> Vassil and Richard, >> >> After this commit, clang errors out when compiling the following code, which >> used to compile without any errors. >> >> $ cat f2.cpp >> extern int compile_time_assert_failed[1]; >> >> template <typename > >> class C { >> enum { D }; >> public: >> template <typename A> void foo1() { >> extern int compile_time_assert_failed[ ((int)C<A>::k > (int)D) ? 1 : -1]; >> } >> }; >> >> template<> >> class C<int> { >> public: >> const static int k = 2; >> }; >> >> void foo2() { >> C<char> c; >> c.foo1<int>(); >> } >> >> $ cat f3.cpp >> void foo1() { >> extern int foo0[1]; >> } >> >> template<int n> >> void foo2() { >> extern int foo0[n ? 1 : -1]; >> } >> >> void foo3() { >> foo2<5>(); >> } >> >> The code looks legal, so I don’t think clang should complain? >> >>> On Feb 28, 2016, at 11:08 AM, Vassil Vassilev via cfe-commits >>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>> >>> Author: vvassilev >>> Date: Sun Feb 28 13:08:24 2016 >>> New Revision: 262189 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=262189&view=rev >>> <http://llvm.org/viewvc/llvm-project?rev=262189&view=rev> >>> Log: >>> [modules] Prefer more complete array types. >>> >>> If we import a module that has a complete array type and one that has an >>> incomplete array type, the declaration found by name lookup might be the >>> one with >>> the incomplete type, possibly resulting in rejects-valid. >>> >>> Now, the name lookup prefers decls with a complete array types. Also, >>> diagnose cases when the redecl chain has array bound, different from the >>> merge >>> candidate. >>> >>> Reviewed by Richard Smith. >>> >>> Modified: >>> cfe/trunk/lib/Sema/SemaDecl.cpp >>> cfe/trunk/lib/Sema/SemaLookup.cpp >>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp >>> cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp >>> 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 >>> >>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=262189&r1=262188&r2=262189&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=262189&r1=262188&r2=262189&view=diff> >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sun Feb 28 13:08:24 2016 >>> @@ -3245,6 +3245,22 @@ void Sema::mergeObjCMethodDecls(ObjCMeth >>> 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 *Ne >>> // 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()) { >>> - 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()) { >>> + 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 = 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 *Ne >>> 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 >>> >>> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=262189&r1=262188&r2=262189&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=262189&r1=262188&r2=262189&view=diff> >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Sun Feb 28 13:08:24 2016 >>> @@ -419,6 +419,18 @@ static bool isPreferredLookupResult(Sema >>> } >>> } >>> >>> + // 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, >>> >>> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=262189&r1=262188&r2=262189&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=262189&r1=262188&r2=262189&view=diff> >>> ============================================================================== >>> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) >>> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Sun Feb 28 13:08:24 2016 >>> @@ -2613,7 +2613,7 @@ static bool isSameEntity(NamedDecl *X, N >>> // 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) >>> >>> Modified: cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp?rev=262189&r1=262188&r2=262189&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp?rev=262189&r1=262188&r2=262189&view=diff> >>> ============================================================================== >>> --- cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp (original) >>> +++ cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp Sun Feb 28 >>> 13:08:24 2016 >>> @@ -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]'}} >>> >>> Modified: cfe/trunk/test/Modules/Inputs/PR26179/A.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/A.h?rev=262189&r1=262188&r2=262189&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/A.h?rev=262189&r1=262188&r2=262189&view=diff> >>> ============================================================================== >>> --- cfe/trunk/test/Modules/Inputs/PR26179/A.h (original) >>> +++ cfe/trunk/test/Modules/Inputs/PR26179/A.h Sun Feb 28 13:08:24 2016 >>> @@ -1,4 +1,2 @@ >>> #include "basic_string.h" >>> #include "B.h" >>> - >>> -int *p = a; >>> >>> Modified: cfe/trunk/test/Modules/Inputs/PR26179/B.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/B.h?rev=262189&r1=262188&r2=262189&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/B.h?rev=262189&r1=262188&r2=262189&view=diff> >>> ============================================================================== >>> --- cfe/trunk/test/Modules/Inputs/PR26179/B.h (original) >>> +++ cfe/trunk/test/Modules/Inputs/PR26179/B.h Sun Feb 28 13:08:24 2016 >>> @@ -1,2 +1 @@ >>> #include "basic_string.h" >>> -extern int a[5]; >>> >>> Modified: 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=262189&r1=262188&r2=262189&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h?rev=262189&r1=262188&r2=262189&view=diff> >>> ============================================================================== >>> --- cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h (original) >>> +++ cfe/trunk/test/Modules/Inputs/PR26179/basic_string.h Sun Feb 28 >>> 13:08:24 2016 >>> @@ -9,6 +9,4 @@ struct basic_string { >>> template<typename T> >>> T basic_string<T>::_S_empty_rep_storage[sizeof(T)]; >>> >>> -extern int a[]; >>> - >>> #endif >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> <http://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
