Author: vvassilev Date: Sun Feb 28 13:08:24 2016 New Revision: 262189 URL: 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 ============================================================================== --- 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 ============================================================================== --- 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 ============================================================================== --- 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 ============================================================================== --- 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 ============================================================================== --- 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 ============================================================================== --- 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 ============================================================================== --- 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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits