Skip to content

Revert "Sema: Remove some unreachable code from CSApply" #36523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,13 @@ ERROR(cannot_apply_lvalue_binop_to_subelement,none,
ERROR(cannot_apply_lvalue_binop_to_rvalue,none,
"left side of mutating operator has immutable type %0", (Type))

ERROR(cannot_subscript_base,none,
"cannot subscript a value of type %0",
(Type))

ERROR(cannot_subscript_ambiguous_base,none,
"cannot subscript a value of incorrect or ambiguous type", ())

ERROR(cannot_subscript_nil_literal,none,
"cannot subscript a nil literal value", ())
ERROR(conditional_cast_from_nil,none,
Expand Down
15 changes: 15 additions & 0 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -3148,9 +3148,24 @@ class LoadExpr : public ImplicitConversionExpr {
static bool classof(const Expr *E) { return E->getKind() == ExprKind::Load; }
};

/// This is a conversion from an expression of UnresolvedType to an arbitrary
/// other type, and from an arbitrary type to UnresolvedType. This node does
/// not appear in valid code, only in code involving diagnostics.
class UnresolvedTypeConversionExpr : public ImplicitConversionExpr {
public:
UnresolvedTypeConversionExpr(Expr *subExpr, Type type)
: ImplicitConversionExpr(ExprKind::UnresolvedTypeConversion, subExpr, type) {}

static bool classof(const Expr *E) {
return E->getKind() == ExprKind::UnresolvedTypeConversion;
}
};

/// FunctionConversionExpr - Convert a function to another function type,
/// which might involve renaming the parameters or handling substitutions
/// of subtypes (in the return) or supertypes (in the input).
///
/// FIXME: This should be a CapturingExpr.
class FunctionConversionExpr : public ImplicitConversionExpr {
public:
FunctionConversionExpr(Expr *subExpr, Type type)
Expand Down
1 change: 1 addition & 0 deletions include/swift/AST/ExprNodes.def
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ ABSTRACT_EXPR(Apply, Expr)
ABSTRACT_EXPR(ImplicitConversion, Expr)
EXPR(Load, ImplicitConversionExpr)
EXPR(DestructureTuple, ImplicitConversionExpr)
EXPR(UnresolvedTypeConversion, ImplicitConversionExpr)
EXPR(FunctionConversion, ImplicitConversionExpr)
EXPR(CovariantFunctionConversion, ImplicitConversionExpr)
EXPR(CovariantReturnConversion, ImplicitConversionExpr)
Expand Down
5 changes: 5 additions & 0 deletions lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2270,6 +2270,11 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
printRec(E->getResultExpr());
PrintWithColorRAII(OS, ParenthesisColor) << ')';
}
void visitUnresolvedTypeConversionExpr(UnresolvedTypeConversionExpr *E) {
printCommon(E, "unresolvedtype_conversion_expr") << '\n';
printRec(E->getSubExpr());
PrintWithColorRAII(OS, ParenthesisColor) << ')';
}
void visitFunctionConversionExpr(FunctionConversionExpr *E) {
printCommon(E, "function_conversion_expr") << '\n';
printRec(E->getSubExpr());
Expand Down
2 changes: 2 additions & 0 deletions lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ ConcreteDeclRef Expr::getReferencedDecl(bool stopAtParenExpr) const {

PASS_THROUGH_REFERENCE(Load, getSubExpr);
NO_REFERENCE(DestructureTuple);
NO_REFERENCE(UnresolvedTypeConversion);
PASS_THROUGH_REFERENCE(FunctionConversion, getSubExpr);
PASS_THROUGH_REFERENCE(CovariantFunctionConversion, getSubExpr);
PASS_THROUGH_REFERENCE(CovariantReturnConversion, getSubExpr);
Expand Down Expand Up @@ -662,6 +663,7 @@ bool Expr::canAppendPostfixExpression(bool appendingPostfixOperator) const {

case ExprKind::Load:
case ExprKind::DestructureTuple:
case ExprKind::UnresolvedTypeConversion:
case ExprKind::FunctionConversion:
case ExprKind::CovariantFunctionConversion:
case ExprKind::CovariantReturnConversion:
Expand Down
8 changes: 8 additions & 0 deletions lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,8 @@ namespace {
RValue visitConditionalBridgeFromObjCExpr(ConditionalBridgeFromObjCExpr *E,
SGFContext C);
RValue visitArchetypeToSuperExpr(ArchetypeToSuperExpr *E, SGFContext C);
RValue visitUnresolvedTypeConversionExpr(UnresolvedTypeConversionExpr *E,
SGFContext C);
RValue visitFunctionConversionExpr(FunctionConversionExpr *E,
SGFContext C);
RValue visitCovariantFunctionConversionExpr(
Expand Down Expand Up @@ -956,6 +958,12 @@ RValue RValueEmitter::visitSuperRefExpr(SuperRefExpr *E, SGFContext C) {
return RValue(SGF, E, result);
}

RValue RValueEmitter::
visitUnresolvedTypeConversionExpr(UnresolvedTypeConversionExpr *E,
SGFContext C) {
llvm_unreachable("invalid code made its way into SILGen");
}

RValue RValueEmitter::visitOtherConstructorDeclRefExpr(
OtherConstructorDeclRefExpr *E, SGFContext C) {
// This should always be a child of an ApplyExpr and so will be emitted by
Expand Down
122 changes: 96 additions & 26 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2884,7 +2884,8 @@ namespace {
auto type = simplifyType(openedType);
cs.setType(expr, type);

assert(!type->is<UnresolvedType>());
if (type->is<UnresolvedType>())
return expr;

auto &ctx = cs.getASTContext();

Expand All @@ -2907,11 +2908,14 @@ namespace {
ConcreteDeclRef witness = conformance.getWitnessByName(
conformingType->getRValueType(), constrName);

auto selectedOverload = solution.getOverloadChoice(
auto selectedOverload = solution.getOverloadChoiceIfAvailable(
cs.getConstraintLocator(expr, ConstraintLocator::ConstructorMember));

if (!selectedOverload)
return nullptr;

auto fnType =
simplifyType(selectedOverload.openedType)->castTo<FunctionType>();
simplifyType(selectedOverload->openedType)->castTo<FunctionType>();

auto newArg = coerceCallArguments(
expr->getArg(), fnType, witness,
Expand Down Expand Up @@ -3211,8 +3215,18 @@ namespace {
// Determine the declaration selected for this overloaded reference.
auto memberLocator = cs.getConstraintLocator(expr,
ConstraintLocator::Member);
auto selected = solution.getOverloadChoice(memberLocator);
auto selectedElt = solution.getOverloadChoiceIfAvailable(memberLocator);

if (!selectedElt) {
// If constraint solving resolved this to an UnresolvedType, then we're
// in an ambiguity tolerant mode used for diagnostic generation. Just
// leave this as whatever type of member reference it already is.
Type resultTy = simplifyType(cs.getType(expr));
cs.setType(expr, resultTy);
return expr;
}

auto selected = *selectedElt;
if (!selected.choice.getBaseType()) {
// This is one of the "outer alternatives", meaning the innermost
// methods didn't work out.
Expand Down Expand Up @@ -3444,19 +3458,40 @@ namespace {
Expr *visitSubscriptExpr(SubscriptExpr *expr) {
auto *memberLocator =
cs.getConstraintLocator(expr, ConstraintLocator::SubscriptMember);
auto overload = solution.getOverloadChoice(memberLocator);
auto overload = solution.getOverloadChoiceIfAvailable(memberLocator);

// Handles situation where there was a solution available but it didn't
// have a proper overload selected from subscript call, might be because
// solver was allowed to return free or unresolved types, which can
// happen while running diagnostics on one of the expressions.
if (!overload) {
auto *base = expr->getBase();
auto &de = cs.getASTContext().Diags;
auto baseType = cs.getType(base);

if (overload.choice.getKind() ==
if (auto errorType = baseType->getAs<ErrorType>()) {
de.diagnose(base->getLoc(), diag::cannot_subscript_base,
errorType->getOriginalType())
.highlight(base->getSourceRange());
} else {
de.diagnose(base->getLoc(), diag::cannot_subscript_ambiguous_base)
.highlight(base->getSourceRange());
}

return nullptr;
}

if (overload->choice.getKind() ==
OverloadChoiceKind::KeyPathDynamicMemberLookup) {
return buildDynamicMemberLookupRef(
expr, expr->getBase(), expr->getIndex()->getStartLoc(), SourceLoc(),
overload, memberLocator);
*overload, memberLocator);
}

return buildSubscript(
expr->getBase(), expr->getIndex(), expr->getArgumentLabels(),
expr->hasTrailingClosure(), cs.getConstraintLocator(expr),
expr->isImplicit(), expr->getAccessSemantics(), overload);
expr->isImplicit(), expr->getAccessSemantics(), *overload);
}

/// "Finish" an array expression by filling in the semantic expression.
Expand Down Expand Up @@ -4327,7 +4362,8 @@ namespace {
expr->getSubPattern()->forEachVariable([](VarDecl *VD) {
VD->setInvalid();
});
if (!SuppressDiagnostics) {
if (!SuppressDiagnostics
&& !cs.getType(simplified)->is<UnresolvedType>()) {
auto &de = cs.getASTContext().Diags;
de.diagnose(simplified->getLoc(), diag::pattern_in_expr,
expr->getSubPattern()->getKind());
Expand Down Expand Up @@ -4806,12 +4842,18 @@ namespace {
// If this is an unresolved link, make sure we resolved it.
if (kind == KeyPathExpr::Component::Kind::UnresolvedProperty ||
kind == KeyPathExpr::Component::Kind::UnresolvedSubscript) {
auto foundDecl = solution.getOverloadChoice(locator);
auto foundDecl = solution.getOverloadChoiceIfAvailable(locator);
if (!foundDecl) {
// If we couldn't resolve the component, leave it alone.
resolvedComponents.push_back(origComponent);
componentTy = origComponent.getComponentType();
continue;
}

isDynamicMember =
foundDecl.choice.getKind() ==
foundDecl->choice.getKind() ==
OverloadChoiceKind::DynamicMemberLookup ||
foundDecl.choice.getKind() ==
foundDecl->choice.getKind() ==
OverloadChoiceKind::KeyPathDynamicMemberLookup;

// If this was a @dynamicMemberLookup property, then we actually
Expand Down Expand Up @@ -4842,9 +4884,11 @@ namespace {
case KeyPathExpr::Component::Kind::OptionalChain: {
didOptionalChain = true;
// Chaining always forces the element to be an rvalue.
assert(!componentTy->hasUnresolvedType());
auto objectTy =
componentTy->getWithoutSpecifierType()->getOptionalObjectType();
if (componentTy->hasUnresolvedType() && !objectTy) {
objectTy = componentTy;
}
assert(objectTy);

auto loc = origComponent.getLoc();
Expand Down Expand Up @@ -4896,8 +4940,8 @@ namespace {
}

// Wrap a non-optional result if there was chaining involved.
assert(!componentTy->hasUnresolvedType());
if (didOptionalChain && componentTy &&
!componentTy->hasUnresolvedType() &&
!componentTy->getWithoutSpecifierType()->isEqual(leafTy)) {
assert(leafTy->getOptionalObjectType()->isEqual(
componentTy->getWithoutSpecifierType()));
Expand All @@ -4916,8 +4960,8 @@ namespace {

// The final component type ought to line up with the leaf type of the
// key path.
assert(!componentTy->hasUnresolvedType());
assert(componentTy->getWithoutSpecifierType()->isEqual(leafTy));
assert(!componentTy || componentTy->hasUnresolvedType()
|| componentTy->getWithoutSpecifierType()->isEqual(leafTy));

if (!isFunctionType)
return E;
Expand Down Expand Up @@ -5022,13 +5066,18 @@ namespace {

// Unwrap the last component type, preserving @lvalue-ness.
auto optionalTy = components.back().getComponentType();
assert(!optionalTy->hasUnresolvedType());
Type objectTy;
if (auto lvalue = optionalTy->getAs<LValueType>()) {
objectTy = lvalue->getObjectType()->getOptionalObjectType();
if (optionalTy->hasUnresolvedType() && !objectTy) {
objectTy = optionalTy;
}
objectTy = LValueType::get(objectTy);
} else {
objectTy = optionalTy->getOptionalObjectType();
if (optionalTy->hasUnresolvedType() && !objectTy) {
objectTy = optionalTy;
}
}
assert(objectTy);

Expand Down Expand Up @@ -5457,9 +5506,11 @@ Expr *ExprRewriter::coerceExistential(Expr *expr, Type toType) {
Type toInstanceType = toType;

// Look through metatypes
while (fromInstanceType->is<AnyMetatypeType>() &&
while ((fromInstanceType->is<UnresolvedType>() ||
fromInstanceType->is<AnyMetatypeType>()) &&
toInstanceType->is<ExistentialMetatypeType>()) {
fromInstanceType = fromInstanceType->castTo<AnyMetatypeType>()->getInstanceType();
if (!fromInstanceType->is<UnresolvedType>())
fromInstanceType = fromInstanceType->castTo<AnyMetatypeType>()->getInstanceType();
toInstanceType = toInstanceType->castTo<ExistentialMetatypeType>()->getInstanceType();
}

Expand Down Expand Up @@ -5659,6 +5710,11 @@ Expr *ExprRewriter::coerceCallArguments(
LocatorPathElt::ApplyArgToParam(argIdx, paramIdx, flags));
};

bool matchCanFail =
llvm::any_of(params, [](const AnyFunctionType::Param &param) {
return param.getPlainType()->hasUnresolvedType();
});

// Determine whether this application has curried self.
bool skipCurriedSelf = apply ? hasCurriedSelf(cs, callee, apply) : true;
// Determine the parameter bindings.
Expand Down Expand Up @@ -5716,7 +5772,9 @@ Expr *ExprRewriter::coerceCallArguments(
args, params, paramInfo, unlabeledTrailingClosureIndex,
/*allowFixes=*/false, listener, trailingClosureMatching);

assert(callArgumentMatch && "Call arguments did not match up?");
assert((matchCanFail || callArgumentMatch) &&
"Call arguments did not match up?");
(void)matchCanFail;

auto parameterBindings = std::move(callArgumentMatch->parameterBindings);

Expand Down Expand Up @@ -6453,7 +6511,8 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
if (knownRestriction != solution.ConstraintRestrictions.end()) {
switch (knownRestriction->second) {
case ConversionRestrictionKind::DeepEquality: {
assert(!toType->hasUnresolvedType());
if (toType->hasUnresolvedType())
break;

// HACK: Fix problem related to Swift 4 mode (with assertions),
// since Swift 4 mode allows passing arguments with extra parens
Expand Down Expand Up @@ -6999,8 +7058,9 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
break;
}

assert(!fromType->hasUnresolvedType());
assert(!toType->hasUnresolvedType());
// Unresolved types come up in diagnostics for lvalue and inout types.
if (fromType->hasUnresolvedType() || toType->hasUnresolvedType())
return cs.cacheType(new (ctx) UnresolvedTypeConversionExpr(expr, toType));

// Use an opaque type to abstract a value of the underlying concrete type.
if (toType->getAs<OpaqueTypeArchetypeType>()) {
Expand Down Expand Up @@ -7577,14 +7637,19 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
// FIXME: handle unwrapping everywhere else
assert(!unwrapResult);

assert(!cs.getType(fn)->is<UnresolvedType>());
// If this is an UnresolvedType in the system, preserve it.
if (cs.getType(fn)->is<UnresolvedType>()) {
cs.setType(apply, cs.getType(fn));
return apply;
}

// We have a type constructor.
auto metaTy = cs.getType(fn)->castTo<AnyMetatypeType>();
auto ty = metaTy->getInstanceType();

// If we're "constructing" a tuple type, it's simply a conversion.
if (auto tupleTy = ty->getAs<TupleType>()) {
// FIXME: Need an AST to represent this properly.
return coerceToType(apply->getArg(), tupleTy, locator);
}

Expand All @@ -7593,14 +7658,19 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
auto *ctorLocator =
cs.getConstraintLocator(locator, {ConstraintLocator::ApplyFunction,
ConstraintLocator::ConstructorMember});
auto selected = solution.getOverloadChoice(ctorLocator);
auto selected = solution.getOverloadChoiceIfAvailable(ctorLocator);
if (!selected) {
assert(ty->hasError() || ty->hasUnresolvedType());
cs.setType(apply, ty);
return apply;
}

assert(ty->getNominalOrBoundGenericNominal() || ty->is<DynamicSelfType>() ||
ty->isExistentialType() || ty->is<ArchetypeType>());

// Consider the constructor decl reference expr 'implicit', but the
// constructor call expr itself has the apply's 'implicitness'.
Expr *declRef = buildMemberRef(fn, /*dotLoc=*/SourceLoc(), selected,
Expr *declRef = buildMemberRef(fn, /*dotLoc=*/SourceLoc(), *selected,
DeclNameLoc(fn->getEndLoc()), locator,
ctorLocator, /*implicit=*/true,
AccessSemantics::Ordinary);
Expand Down
3 changes: 3 additions & 0 deletions validation-test/IDE/crashers_2_fixed/rdar75200446.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=COMPLETE

.undefined() #^COMPLETE^#
3 changes: 3 additions & 0 deletions validation-test/IDE/crashers_2_fixed/rdar75219757.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=COMPLETE

0odd.foo.bar #^COMPLETE^#