Skip to content

Commit caeed32

Browse files
committed
Add a fix-it for missing generic parameters on construction.
For example, if someone tries to use the newly-generic type Cache, from Foundation: var cache = Cache() they'll now get a fix-it to substitute the default generic parameters: var cache = Cache<AnyObject, AnyObject>() The rules for choosing this placeholder type are based on constraints and won't be right 100% of the time, but they should be reasonable. (In particular, constraints on associated types are ignored.) In cases where there's no one concrete type that will work, an Xcode- style placeholder is inserted instead. - An unconstrained generic parameter defaults to 'Any'. - A superclass-constrained parameter defaults to that class, e.g. 'UIView'. - A parameter constrained to a single @objc protocol (or to AnyObject) defaults to that protocol, e.g. 'NSCoding'. - Anything else gets a placeholder using the generic parameter's name and protocol composition syntax. rdar://problem/27087345
1 parent f21cf8b commit caeed32

File tree

9 files changed

+293
-33
lines changed

9 files changed

+293
-33
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,8 @@ ERROR(unbound_generic_parameter_cast,none,
852852
"generic parameter %0 could not be inferred in cast to %1", (Type, Type))
853853
NOTE(archetype_declared_in_type,none,
854854
"%0 declared as parameter to type %1", (Type, Type))
855+
NOTE(unbound_generic_parameter_explicit_fix,none,
856+
"explicitly specify the generic arguments to fix this issue", ())
855857

856858

857859
ERROR(string_index_not_integer,none,

lib/Sema/CSDiag.cpp

Lines changed: 92 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6491,39 +6491,112 @@ void ConstraintSystem::diagnoseFailureForExpr(Expr *expr) {
64916491
diagnosis.diagnoseAmbiguity(expr);
64926492
}
64936493

6494+
static bool hasArchetype(const GenericTypeDecl *generic,
6495+
const ArchetypeType *archetype) {
6496+
return std::any_of(generic->getInnermostGenericParamTypes().begin(),
6497+
generic->getInnermostGenericParamTypes().end(),
6498+
[archetype](const GenericTypeParamType *genericParamTy) {
6499+
return genericParamTy->getDecl()->getArchetype() == archetype;
6500+
});
6501+
}
6502+
64946503
static void noteArchetypeSource(const TypeLoc &loc, ArchetypeType *archetype,
6495-
TypeChecker &tc) {
6496-
GenericTypeDecl *FoundDecl = nullptr;
6497-
6504+
ConstraintSystem &cs) {
6505+
const GenericTypeDecl *FoundDecl = nullptr;
6506+
const ComponentIdentTypeRepr *FoundGenericTypeBase = nullptr;
6507+
64986508
// Walk the TypeRepr to find the type in question.
64996509
if (auto typerepr = loc.getTypeRepr()) {
65006510
struct FindGenericTypeDecl : public ASTWalker {
6501-
GenericTypeDecl *&FoundDecl;
6502-
FindGenericTypeDecl(GenericTypeDecl *&FoundDecl) : FoundDecl(FoundDecl) {
6503-
}
6511+
const GenericTypeDecl *FoundDecl = nullptr;
6512+
const ComponentIdentTypeRepr *FoundGenericTypeBase = nullptr;
6513+
const ArchetypeType *Archetype;
6514+
6515+
FindGenericTypeDecl(const ArchetypeType *Archetype)
6516+
: Archetype(Archetype) {}
65046517

65056518
bool walkToTypeReprPre(TypeRepr *T) override {
65066519
// If we already emitted the note, we're done.
65076520
if (FoundDecl) return false;
65086521

6509-
if (auto ident = dyn_cast<ComponentIdentTypeRepr>(T))
6510-
FoundDecl = dyn_cast_or_null<GenericTypeDecl>(ident->getBoundDecl());
6522+
if (auto ident = dyn_cast<ComponentIdentTypeRepr>(T)) {
6523+
auto *generic =
6524+
dyn_cast_or_null<GenericTypeDecl>(ident->getBoundDecl());
6525+
if (hasArchetype(generic, Archetype)) {
6526+
FoundDecl = generic;
6527+
FoundGenericTypeBase = ident;
6528+
return false;
6529+
}
6530+
}
65116531
// Keep walking.
65126532
return true;
65136533
}
6514-
} findGenericTypeDecl(FoundDecl);
6515-
6534+
} findGenericTypeDecl(archetype);
6535+
65166536
typerepr->walk(findGenericTypeDecl);
6537+
FoundDecl = findGenericTypeDecl.FoundDecl;
6538+
FoundGenericTypeBase = findGenericTypeDecl.FoundGenericTypeBase;
65176539
}
6518-
6540+
65196541
// If we didn't find the type in the TypeRepr, fall back to the type in the
65206542
// type checked expression.
6521-
if (!FoundDecl)
6522-
FoundDecl = loc.getType()->getAnyGeneric();
6523-
6524-
if (FoundDecl)
6525-
tc.diagnose(FoundDecl, diag::archetype_declared_in_type, archetype,
6526-
FoundDecl->getDeclaredType());
6543+
if (!FoundDecl) {
6544+
if (const GenericTypeDecl *generic = loc.getType()->getAnyGeneric())
6545+
if (hasArchetype(generic, archetype))
6546+
FoundDecl = generic;
6547+
}
6548+
6549+
auto &tc = cs.getTypeChecker();
6550+
if (FoundDecl) {
6551+
tc.diagnose(FoundDecl, diag::archetype_declared_in_type,
6552+
archetype, FoundDecl->getDeclaredType());
6553+
}
6554+
6555+
if (FoundGenericTypeBase && !isa<GenericIdentTypeRepr>(FoundGenericTypeBase)){
6556+
assert(FoundDecl);
6557+
6558+
// If we can, prefer using any types already fixed by the constraint system.
6559+
// This lets us produce fixes like `Pair<Int, Any>` instead of defaulting to
6560+
// `Pair<Any, Any>`.
6561+
// Right now we only handle this when the type that's at fault is the
6562+
// top-level type passed to this function.
6563+
ArrayRef<Type> genericArgs;
6564+
if (auto *boundGenericTy = loc.getType()->getAs<BoundGenericType>()) {
6565+
if (boundGenericTy->getDecl() == FoundDecl)
6566+
genericArgs = boundGenericTy->getGenericArgs();
6567+
}
6568+
6569+
auto getPreferredType =
6570+
[&](const GenericTypeParamDecl *genericParam) -> Type {
6571+
// If we were able to get the generic arguments (i.e. the types used at
6572+
// FoundDecl's use site), we can prefer those...
6573+
if (genericArgs.empty())
6574+
return Type();
6575+
6576+
Type preferred = genericArgs[genericParam->getIndex()];
6577+
if (!preferred || preferred->is<ErrorType>())
6578+
return Type();
6579+
6580+
// ...but only if they were actually resolved by the constraint system
6581+
// despite the failure.
6582+
TypeVariableType *unused;
6583+
Type maybeFixedType = cs.getFixedTypeRecursive(preferred, unused,
6584+
/*wantRValue*/true);
6585+
if (maybeFixedType->hasTypeVariable() ||
6586+
maybeFixedType->hasUnresolvedType()) {
6587+
return Type();
6588+
}
6589+
return maybeFixedType;
6590+
};
6591+
6592+
SmallString<64> genericParamBuf;
6593+
if (tc.getDefaultGenericArgumentsString(genericParamBuf, FoundDecl,
6594+
getPreferredType)) {
6595+
tc.diagnose(FoundGenericTypeBase->getLoc(),
6596+
diag::unbound_generic_parameter_explicit_fix)
6597+
.fixItInsertAfter(FoundGenericTypeBase->getEndLoc(), genericParamBuf);
6598+
}
6599+
}
65276600
}
65286601

65296602

@@ -6639,11 +6712,7 @@ void FailureDiagnosis::diagnoseUnboundArchetype(ArchetypeType *archetype,
66396712
.highlight(ECE->getCastTypeLoc().getSourceRange());
66406713

66416714
// Emit a note specifying where this came from, if we can find it.
6642-
noteArchetypeSource(ECE->getCastTypeLoc(), archetype, tc);
6643-
if (auto *ND = ECE->getCastTypeLoc().getType()
6644-
->getNominalOrBoundGenericNominal())
6645-
tc.diagnose(ND, diag::archetype_declared_in_type, archetype,
6646-
ND->getDeclaredType());
6715+
noteArchetypeSource(ECE->getCastTypeLoc(), archetype, *CS);
66476716
return;
66486717
}
66496718

@@ -6673,7 +6742,7 @@ void FailureDiagnosis::diagnoseUnboundArchetype(ArchetypeType *archetype,
66736742

66746743
if (auto TE = dyn_cast<TypeExpr>(anchor)) {
66756744
// Emit a note specifying where this came from, if we can find it.
6676-
noteArchetypeSource(TE->getTypeLoc(), archetype, tc);
6745+
noteArchetypeSource(TE->getTypeLoc(), archetype, *CS);
66776746
return;
66786747
}
66796748

lib/Sema/MiscDiagnostics.cpp

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,69 @@ static void diagnoseImplicitSelfUseInClosure(TypeChecker &TC, const Expr *E,
932932
const_cast<Expr *>(E)->walk(DiagnoseWalker(TC, isAlreadyInClosure));
933933
}
934934

935+
bool TypeChecker::getDefaultGenericArgumentsString(
936+
SmallVectorImpl<char> &buf,
937+
const swift::GenericTypeDecl *typeDecl,
938+
llvm::function_ref<Type(const GenericTypeParamDecl *)> getPreferredType) {
939+
llvm::raw_svector_ostream genericParamText{buf};
940+
genericParamText << "<";
941+
942+
auto printGenericParamSummary =
943+
[&](const GenericTypeParamType *genericParamTy) {
944+
const GenericTypeParamDecl *genericParam = genericParamTy->getDecl();
945+
if (Type result = getPreferredType(genericParam)) {
946+
result.print(genericParamText);
947+
return;
948+
}
949+
950+
ArrayRef<ProtocolDecl *> protocols =
951+
genericParam->getConformingProtocols(this);
952+
953+
if (Type superclass = genericParam->getSuperclass()) {
954+
if (protocols.empty()) {
955+
superclass.print(genericParamText);
956+
return;
957+
}
958+
959+
genericParamText << "<#" << genericParam->getName() << ": ";
960+
superclass.print(genericParamText);
961+
for (const ProtocolDecl *proto : protocols) {
962+
if (proto->isSpecificProtocol(KnownProtocolKind::AnyObject))
963+
continue;
964+
genericParamText << " & " << proto->getName();
965+
}
966+
genericParamText << "#>";
967+
return;
968+
}
969+
970+
if (protocols.empty()) {
971+
genericParamText << Context.Id_Any;
972+
return;
973+
}
974+
975+
if (protocols.size() == 1 &&
976+
(protocols.front()->isObjC() ||
977+
protocols.front()->isSpecificProtocol(KnownProtocolKind::AnyObject))) {
978+
genericParamText << protocols.front()->getName();
979+
return;
980+
}
981+
982+
genericParamText << "<#" << genericParam->getName() << ": ";
983+
interleave(protocols,
984+
[&](const ProtocolDecl *proto) {
985+
genericParamText << proto->getName();
986+
},
987+
[&] { genericParamText << " & "; });
988+
genericParamText << "#>";
989+
};
990+
991+
interleave(typeDecl->getInnermostGenericParamTypes(),
992+
printGenericParamSummary, [&]{ genericParamText << ", "; });
993+
994+
genericParamText << ">";
995+
return true;
996+
}
997+
935998
/// Diagnose an argument labeling issue, returning true if we successfully
936999
/// diagnosed the issue.
9371000
bool swift::diagnoseArgumentLabelError(TypeChecker &TC, const Expr *expr,

lib/Sema/TypeChecker.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1980,6 +1980,23 @@ class TypeChecker final : public LazyResolver {
19801980
ConstructorDecl *ctorDecl,
19811981
bool SuppressDiagnostics);
19821982

1983+
/// Builds a string representing a "default" generic argument list for
1984+
/// \p typeDecl. In general, this means taking the bound of each generic
1985+
/// parameter. The \p getPreferredType callback can be used to provide a
1986+
/// different type from the bound.
1987+
///
1988+
/// It may not always be possible to find a single appropriate type for a
1989+
/// particular parameter (say, if it has two bounds). In this case, an
1990+
/// Xcode-style placeholder will be used instead.
1991+
///
1992+
/// Returns true if the arguments list could be constructed, false if for
1993+
/// some reason it could not.
1994+
bool getDefaultGenericArgumentsString(
1995+
SmallVectorImpl<char> &buf,
1996+
const GenericTypeDecl *typeDecl,
1997+
llvm::function_ref<Type(const GenericTypeParamDecl *)> getPreferredType =
1998+
[](const GenericTypeParamDecl *) { return Type(); });
1999+
19832000
/// Attempt to omit needless words from the name of the given declaration.
19842001
Optional<DeclName> omitNeedlessWords(AbstractFunctionDecl *afd);
19852002

test/Constraints/bridging.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ func rdar20029786(_ ns: NSString?) {
298298

299299
// <rdar://problem/19813772> QoI: Using as! instead of as in this case produces really bad diagnostic
300300
func rdar19813772(_ nsma: NSMutableArray) {
301-
var a1 = nsma as! Array // expected-error{{generic parameter 'Element' could not be inferred in cast to 'Array<_>'}}
301+
var a1 = nsma as! Array // expected-error{{generic parameter 'Element' could not be inferred in cast to 'Array<_>'}} expected-note {{explicitly specify the generic arguments to fix this issue}} {{26-26=<Any>}}
302302
// FIXME: The following diagnostic is misleading and should not happen: expected-warning@-1{{cast from 'NSMutableArray' to unrelated type 'Array<_>' always fails}}
303303
var a2 = nsma as! Array<AnyObject> // expected-warning{{forced cast from 'NSMutableArray' to 'Array<AnyObject>' always succeeds; did you mean to use 'as'?}} {{17-20=as}}
304304
var a3 = nsma as Array<AnyObject>

test/Constraints/construction.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ acceptString("\(hello), \(world) #\(i)!")
5757
Optional<Int>(1) // expected-warning{{unused}}
5858
Optional(1) // expected-warning{{unused}}
5959
_ = .none as Optional<Int>
60-
Optional(.none) // expected-error{{generic parameter 'T' could not be inferred}}
60+
Optional(.none) // expected-error{{generic parameter 'T' could not be inferred}} expected-note {{explicitly specify the generic arguments to fix this issue}} {{9-9=<Any>}}
6161

6262
// Interpolation
6363
_ = "\(hello), \(world) #\(i)!"

0 commit comments

Comments
 (0)