diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 61ec1a775290..4a1bbde271d2 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -3245,6 +3245,22 @@ void Sema::mergeObjCMethodDecls(ObjCMethodDecl *newMethod, 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 *New, VarDecl *Old, // 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()) { + 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 = New->getType(); - } else if (Old->getType()->isArrayType() && - New->getType()->isIncompleteArrayType()) { - 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 *New, VarDecl *Old, 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 diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index a35c7600ab55..4baf6053359c 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/clang/lib/Sema/SemaLookup.cpp @@ -419,6 +419,18 @@ static bool isPreferredLookupResult(Sema &S, Sema::LookupNameKind Kind, } } + // VarDecl can have incomplete array types, prefer the one with more complete + // array type. + if (VarDecl *DVD = dyn_cast(DUnderlying)) { + VarDecl *EVD = cast(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(DUnderlying) && !isa(DUnderlying)) { // If the existing declaration is hidden, prefer the new one. Otherwise, diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 84dbe3a796f3..f484345dfc50 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -2613,7 +2613,7 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) { // template struct S { static T Var[]; }; // #1 // template T S::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) diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp index 4686b1c961ec..188a0a2c7eed 100644 --- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp +++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp @@ -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]'}} diff --git a/clang/test/Modules/Inputs/PR26179/A.h b/clang/test/Modules/Inputs/PR26179/A.h index ce95fafb4bf0..c264f4cf9bfb 100644 --- a/clang/test/Modules/Inputs/PR26179/A.h +++ b/clang/test/Modules/Inputs/PR26179/A.h @@ -1,4 +1,2 @@ #include "basic_string.h" #include "B.h" - -int *p = a; diff --git a/clang/test/Modules/Inputs/PR26179/B.h b/clang/test/Modules/Inputs/PR26179/B.h index eb8d1c29e795..46a109efdb59 100644 --- a/clang/test/Modules/Inputs/PR26179/B.h +++ b/clang/test/Modules/Inputs/PR26179/B.h @@ -1,2 +1 @@ #include "basic_string.h" -extern int a[5]; diff --git a/clang/test/Modules/Inputs/PR26179/basic_string.h b/clang/test/Modules/Inputs/PR26179/basic_string.h index afd1e0d3b340..653ce078451c 100644 --- a/clang/test/Modules/Inputs/PR26179/basic_string.h +++ b/clang/test/Modules/Inputs/PR26179/basic_string.h @@ -9,6 +9,4 @@ struct basic_string { template T basic_string::_S_empty_rep_storage[sizeof(T)]; -extern int a[]; - #endif