Skip to content

Commit

Permalink
Revert all the function type ABI restriction changes.
Browse files Browse the repository at this point in the history 
John pointed out that messing with the type checker's notion of "subtype"
is a bad idea. Instead, we should just have a separate check for ABI
compatibility...and eventually (rdar://problem/19517003) just insert the
appropriate thunks rather than forcing the user to perform the conversion.

I'm leaving all the tests as they are because I'm adding a post-type-checking
diagnostic in the next commit, and that should pass all the same tests.

Part of rdar://problem/19600325

Swift SVN r25116
  • Loading branch information
@jrose-apple
jrose-apple committed Feb 10, 2015
1 parent d697856 commit 07041fc
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 189 deletions.
63 changes: 0 additions & 63 deletions lib/Sema/CSApply.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5141,20 +5141,6 @@ Expr *ConstraintSystem::applySolution(Solution &solution, Expr *expr,
}
}

// Fixing an invalid function conversion comes from a mismatch on the
// input or result type, which we strip.
if (fix.first.getKind() == FixKind::FunctionConversion) {
auto path = locator->getPath();
if (path.empty())
continue;
if (path.back().getKind() != ConstraintLocator::FunctionArgument &&
path.back().getKind() != ConstraintLocator::FunctionResult)
continue;
path = path.drop_back();
auto newFlags = ConstraintLocator::getSummaryFlagsForPath(path);
locator = getConstraintLocator(locator->getAnchor(), path, newFlags);
}

// Resolve the locator to a specific expression.
SourceRange range1, range2;
ConstraintLocator *resolved
Expand Down Expand Up @@ -5384,55 +5370,6 @@ Expr *ConstraintSystem::applySolution(Solution &solution, Expr *expr,
break;
}

case FixKind::FunctionConversion: {
Type fromType =
solution.simplifyType(TC, affected->getType())->getRValueObjectType();
Type toType = solution.simplifyType(TC,
fix.first.getTypeArgument(*this));

// Strip @noescape from the destination type. It's not relevant: any
// function type with otherwise matching flags can be converted to a
// noescape function type.
auto toFnType = toType->castTo<AnyFunctionType>();
auto prettyExtInfo = toFnType->getExtInfo().withNoEscape(false);
toType = toFnType->withExtInfo(prettyExtInfo);

TC.diagnose(affected->getLoc(), diag::invalid_function_conversion,
fromType, toType);
diagnosed = true;

if (!isa<ClosureExpr>(affected) && !isa<CaptureListExpr>(affected)) {
// If the function value we're converting isn't a closure, suggest
// wrapping it in one
bool needsParens = !affected->canAppendCallParentheses();

SourceLoc endLoc = Lexer::getLocForEndOfToken(TC.Context.SourceMgr,
affected->getEndLoc());
auto activeDiag =
TC.diagnose(affected->getLoc(),
diag::invalid_function_conversion_closure);

auto hasAnyLabeledParams = [](const AnyFunctionType *fnTy) -> bool {
auto *params = fnTy->getInput()->getAs<TupleType>();
if (!params)
return false;
return std::any_of(params->getFields().begin(),
params->getFields().end(),
[](TupleTypeElt field) {
return field.hasName();
});
};

if (!hasAnyLabeledParams(toType->castTo<AnyFunctionType>())) {
activeDiag.fixItInsert(affected->getStartLoc(),
needsParens ? "{ (" : "{ ");
activeDiag.fixItInsert(endLoc, needsParens ? ")($0) }" : "($0) }");
}
}

break;
}

case FixKind::CoerceToCheckedCast: {
if (auto *coerceExpr = dyn_cast<CoerceExpr>(locator->getAnchor())) {
Expr *subExpr = coerceExpr->getSubExpr();
Expand Down
186 changes: 84 additions & 102 deletions lib/Sema/CSSimplify.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,6 @@ matchCallArguments(ConstraintSystem &cs, TypeMatchKind kind,
case TypeMatchKind::BindToPointerType:
case TypeMatchKind::SameType:
case TypeMatchKind::Subtype:
case TypeMatchKind::MetatypeSubtype:
llvm_unreachable("Not an call argument constraint");
}

Expand Down Expand Up @@ -783,7 +782,6 @@ ConstraintSystem::matchTupleTypes(TupleType *tuple1, TupleType *tuple2,
case TypeMatchKind::BindToPointerType:
case TypeMatchKind::SameType:
case TypeMatchKind::Subtype:
case TypeMatchKind::MetatypeSubtype:
llvm_unreachable("Not a conversion");
}

Expand Down Expand Up @@ -910,42 +908,6 @@ static bool isFunctionTypeAcceptingNoArguments(Type type) {
return true;
}

/// \brief Map a type-matching kind to a constraint kind.
static ConstraintKind getConstraintKind(TypeMatchKind kind) {
switch (kind) {
case TypeMatchKind::BindType:
case TypeMatchKind::BindToPointerType:
return ConstraintKind::Bind;

case TypeMatchKind::SameType:
return ConstraintKind::Equal;

case TypeMatchKind::Subtype:
case TypeMatchKind::MetatypeSubtype:
return ConstraintKind::Subtype;

case TypeMatchKind::Conversion:
return ConstraintKind::Conversion;

case TypeMatchKind::ExplicitConversion:
return ConstraintKind::ExplicitConversion;

case TypeMatchKind::ArgumentConversion:
return ConstraintKind::ArgumentConversion;

case TypeMatchKind::ArgumentTupleConversion:
return ConstraintKind::ArgumentTupleConversion;

case TypeMatchKind::OperatorArgumentTupleConversion:
return ConstraintKind::OperatorArgumentTupleConversion;

case TypeMatchKind::OperatorArgumentConversion:
return ConstraintKind::OperatorArgumentConversion;
}

llvm_unreachable("unhandled type matching kind");
}

ConstraintSystem::SolutionKind
ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
TypeMatchKind kind, unsigned flags,
Expand Down Expand Up @@ -1003,55 +965,53 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
}

// Determine how we match up the input/result types.
TypeMatchKind subKind = std::min(kind, TypeMatchKind::Subtype);
TypeMatchKind subKind;
switch (kind) {
case TypeMatchKind::BindType:
case TypeMatchKind::BindToPointerType:
case TypeMatchKind::SameType:
subKind = kind;
break;

case TypeMatchKind::Subtype:
case TypeMatchKind::Conversion:
case TypeMatchKind::ExplicitConversion:
case TypeMatchKind::ArgumentConversion:
case TypeMatchKind::ArgumentTupleConversion:
case TypeMatchKind::OperatorArgumentTupleConversion:
case TypeMatchKind::OperatorArgumentConversion:
subKind = TypeMatchKind::Subtype;
break;
}

unsigned subFlags = flags | TMF_GenerateConstraints;

// Input types can be contravariant (or equal).
auto *inputLoc = getConstraintLocator(locator.withPathElement(
ConstraintLocator::FunctionArgument));
auto *inputTypeMatch = Constraint::create(*this, getConstraintKind(subKind),
func2->getInput(),
func1->getInput(),
{}, inputLoc);

auto *resultLoc = getConstraintLocator(locator.withPathElement(
ConstraintLocator::FunctionResult));
auto *resultTypeMatch = Constraint::create(*this, getConstraintKind(subKind),
func1->getResult(),
func2->getResult(),
{}, resultLoc);

if (!shouldAttemptFixes() || kind < TypeMatchKind::Conversion) {
addConstraint(inputTypeMatch);
addConstraint(resultTypeMatch);
return SolutionKind::Solved;
SolutionKind result = matchTypes(func2->getInput(), func1->getInput(),
subKind, subFlags,
locator.withPathElement(
ConstraintLocator::FunctionArgument));
if (result == SolutionKind::Error)
return SolutionKind::Error;

// Result type can be covariant (or equal).
switch (matchTypes(func1->getResult(), func2->getResult(), subKind,
subFlags,
locator.withPathElement(
ConstraintLocator::FunctionResult))) {
case SolutionKind::Error:
return SolutionKind::Error;

case SolutionKind::Solved:
result = SolutionKind::Solved;
break;

case SolutionKind::Unsolved:
result = SolutionKind::Unsolved;
break;
}

// If we're trying to fix the constraint system, consider allowing a
// function conversion that changes the ABI.
auto *matchLoc = getConstraintLocator(locator);
auto *subtypeMatch =
Constraint::createConjunction(*this, {inputTypeMatch, resultTypeMatch},
matchLoc);

TypeMatchKind fixKind = std::min(kind, TypeMatchKind::Conversion);
auto *inputConversion =
Constraint::createFixed(*this, getConstraintKind(fixKind),
Fix::getFunctionConversion(*this, func2),
func2->getInput(), func1->getInput(), inputLoc);

auto *resultConversion = Constraint::create(*this, getConstraintKind(fixKind),
func1->getResult(),
func2->getResult(),
{}, resultLoc);
auto *subtypeConversion =
Constraint::createConjunction(*this, {inputConversion, resultConversion},
matchLoc);

auto *disjunction =
Constraint::createDisjunction(*this, {subtypeMatch, subtypeConversion},
matchLoc);
addConstraint(disjunction);
return SolutionKind::Solved;
return result;
}

/// \brief Map a failed type-matching kind to a failure kind, generically.
Expand All @@ -1063,7 +1023,6 @@ static Failure::FailureKind getRelationalFailureKind(TypeMatchKind kind) {
return Failure::TypesNotEqual;

case TypeMatchKind::Subtype:
case TypeMatchKind::MetatypeSubtype:
return Failure::TypesNotSubtypes;

case TypeMatchKind::Conversion:
Expand Down Expand Up @@ -1189,6 +1148,41 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2,
return SolutionKind::Solved;
}

/// \brief Map a type-matching kind to a constraint kind.
static ConstraintKind getConstraintKind(TypeMatchKind kind) {
switch (kind) {
case TypeMatchKind::BindType:
case TypeMatchKind::BindToPointerType:
return ConstraintKind::Bind;

case TypeMatchKind::SameType:
return ConstraintKind::Equal;

case TypeMatchKind::Subtype:
return ConstraintKind::Subtype;

case TypeMatchKind::Conversion:
return ConstraintKind::Conversion;

case TypeMatchKind::ExplicitConversion:
return ConstraintKind::ExplicitConversion;

case TypeMatchKind::ArgumentConversion:
return ConstraintKind::ArgumentConversion;

case TypeMatchKind::ArgumentTupleConversion:
return ConstraintKind::ArgumentTupleConversion;

case TypeMatchKind::OperatorArgumentTupleConversion:
return ConstraintKind::OperatorArgumentTupleConversion;

case TypeMatchKind::OperatorArgumentConversion:
return ConstraintKind::OperatorArgumentConversion;
}

llvm_unreachable("unhandled type matching kind");
}

static bool isStringCompatiblePointerBaseType(TypeChecker &TC,
DeclContext *DC,
Type baseType) {
Expand Down Expand Up @@ -1383,7 +1377,6 @@ ConstraintSystem::matchTypes(Type type1, Type type2, TypeMatchKind kind,
}

case TypeMatchKind::Subtype:
case TypeMatchKind::MetatypeSubtype:
case TypeMatchKind::Conversion:
case TypeMatchKind::ExplicitConversion:
case TypeMatchKind::ArgumentConversion:
Expand Down Expand Up @@ -1536,7 +1529,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, TypeMatchKind kind,
kind != TypeMatchKind::SameType &&
(meta1->getInstanceType()->mayHaveSuperclass() ||
meta2->getInstanceType()->getClassOrBoundGenericClass()))
subKind = std::min(kind, TypeMatchKind::MetatypeSubtype);
subKind = std::min(kind, TypeMatchKind::Subtype);

return matchTypes(meta1->getInstanceType(), meta2->getInstanceType(),
subKind, subFlags,
Expand Down Expand Up @@ -1879,12 +1872,12 @@ ConstraintSystem::matchTypes(Type type1, Type type2, TypeMatchKind kind,

// For a subtyping relation involving a conversion to an existential
// metatype, match the child types.
if (concrete && kind >= TypeMatchKind::Conversion) {
if (concrete && kind >= TypeMatchKind::Subtype) {
if (auto meta1 = type1->getAs<MetatypeType>()) {
if (auto meta2 = type2->getAs<ExistentialMetatypeType>()) {
return matchTypes(meta1->getInstanceType(),
meta2->getInstanceType(),
TypeMatchKind::MetatypeSubtype, subFlags,
TypeMatchKind::Subtype, subFlags,
locator.withPathElement(
ConstraintLocator::InstanceType));
}
Expand All @@ -1897,11 +1890,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, TypeMatchKind kind,
// protocols in the second type.
if (concrete && kind >= TypeMatchKind::Subtype &&
type2->isExistentialType()) {

if (kind >= TypeMatchKind::MetatypeSubtype ||
(type1->isAnyClassReferenceType() && type2->isClassExistentialType())) {
conversionsOrFixes.push_back(ConversionRestrictionKind::Existential);
}
conversionsOrFixes.push_back(ConversionRestrictionKind::Existential);
}

// A value of type T can be converted to type U? if T is convertible to U.
Expand Down Expand Up @@ -1941,11 +1930,8 @@ ConstraintSystem::matchTypes(Type type1, Type type2, TypeMatchKind kind,
// Do not attempt a value-to-optional conversion when resolving the
// applicable overloads for an operator application with nil operands.
if (!(subFlags & TMF_ApplyingOperatorWithNil)) {
if (kind >= TypeMatchKind::Conversion ||
type1->isAnyClassReferenceType()) {
conversionsOrFixes.push_back(
ConversionRestrictionKind::ValueToOptional);
}
conversionsOrFixes.push_back(
ConversionRestrictionKind::ValueToOptional);
}
}
}
Expand Down Expand Up @@ -4293,10 +4279,6 @@ ConstraintSystem::simplifyFixConstraint(Fix fix,
case FixKind::ToRawToRawValue:
case FixKind::CoerceToCheckedCast:
llvm_unreachable("handled elsewhere");

case FixKind::FunctionConversion:
// We already have a different matchKind.
return matchTypes(type1, type2, matchKind, subFlags, locator);
}
}

Expand Down
12 changes: 1 addition & 11 deletions lib/Sema/Constraint.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,21 +409,13 @@ Fix Fix::getForcedDowncast(ConstraintSystem &cs, Type toType) {
return Fix(FixKind::ForceDowncast, index);
}

Fix Fix::getFunctionConversion(ConstraintSystem &cs,
FunctionType *toType) {
unsigned index = cs.FixedTypes.size();
cs.FixedTypes.push_back(toType);
return Fix(FixKind::FunctionConversion, index);
}

ArrayRef<Identifier> Fix::getRelabelTupleNames(ConstraintSystem &cs) const {
assert(isRelabelTuple());
return cs.RelabelTupleNames[Data];
}

Type Fix::getTypeArgument(ConstraintSystem &cs) const {
assert(getKind() == FixKind::ForceDowncast ||
getKind() == FixKind::FunctionConversion);
assert(getKind() == FixKind::ForceDowncast);
return cs.FixedTypes[Data];
}

Expand Down Expand Up @@ -453,8 +445,6 @@ StringRef Fix::getName(FixKind kind) {
return "fix: fromRaw(x) to init(rawValue:x)";
case FixKind::ToRawToRawValue:
return "fix: toRaw() to rawValue";
case FixKind::FunctionConversion:
return "fix: function conversion";
case FixKind::CoerceToCheckedCast:
return "fix: as to as!";
}
Expand Down
Loading

0 comments on commit 07041fc

Please sign in to comment.