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
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 <mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
