Skip to content

Improve type join for function types. #15365

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 1 commit into from
Mar 21, 2018
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
43 changes: 32 additions & 11 deletions lib/AST/TypeJoinMeet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ struct TypeJoin : CanTypeVisitor<TypeJoin, CanType> {
// implementation.
CanType Unimplemented;

// Always null. Used as a marker for places where there is no join
// of two types in our type system.
CanType Nonexistent;

// For convenience, TheAnyType from ASTContext;
CanType TheAnyType;

Expand Down Expand Up @@ -98,14 +102,13 @@ struct TypeJoin : CanTypeVisitor<TypeJoin, CanType> {
if (second->getOptionalObjectType())
return TypeJoin(first).visit(second);

// Likewise, rather than making every visitor deal with Any, just
// handle it here.
// join with Any is always Any
// Likewise, rather than making every visitor deal with Any,
// always dispatch to the protocol composition side of the join.
if (first->isAny())
return first;
return TypeJoin(second).visit(first);

if (second->isAny())
return second;
return TypeJoin(first).visit(second);

// Otherwise the first type might be an optional (or not), so
// dispatch there.
Expand Down Expand Up @@ -301,14 +304,23 @@ CanType TypeJoin::visitDependentMemberType(CanType second) {
CanType TypeJoin::visitFunctionType(CanType second) {
assert(First != second);

if (First->getKind() != second->getKind())
return TheAnyType;
auto secondFnTy = second->castTo<FunctionType>();

if (First->getKind() != second->getKind()) {
if (secondFnTy->getExtInfo().isNoEscape()) {
return Nonexistent;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just return CanType()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the comment for this member calls out, this makes it clear why the value is being returned.

} else {
return TheAnyType;
}
}

auto firstFnTy = First->castTo<FunctionType>();
auto secondFnTy = second->castTo<FunctionType>();

auto firstExtInfo = firstFnTy->getExtInfo();
auto secondExtInfo = secondFnTy->getExtInfo();

// FIXME: Properly handle these attributes.
if (firstFnTy->getExtInfo() != secondFnTy->getExtInfo())
if (firstExtInfo.withNoEscape(false) != secondExtInfo.withNoEscape(false))
return Unimplemented;

// FIXME: Properly compute parameter types from getParams().
Expand All @@ -323,8 +335,12 @@ CanType TypeJoin::visitFunctionType(CanType second) {
if (!result)
return Unimplemented;

auto extInfo = firstExtInfo;
if (secondFnTy->getExtInfo().isNoEscape())
extInfo = extInfo.withNoEscape(true);

return FunctionType::get(firstFnTy->getInput(), result,
firstFnTy->getExtInfo())->getCanonicalType();
extInfo)->getCanonicalType();
}

CanType TypeJoin::visitGenericFunctionType(CanType second) {
Expand All @@ -337,8 +353,13 @@ CanType TypeJoin::visitGenericFunctionType(CanType second) {
}

CanType TypeJoin::visitProtocolCompositionType(CanType second) {
if (second->isAny())
if (second->isAny()) {
auto *fnTy = First->getAs<AnyFunctionType>();
if (fnTy && fnTy->getExtInfo().isNoEscape())
return Nonexistent;

return second;
}

return Unimplemented;
}
Expand Down
27 changes: 26 additions & 1 deletion lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2954,7 +2954,12 @@ namespace {
llvm_unreachable("found KeyPathDotExpr in CSGen");
}

enum class TypeOperation { None, Join, JoinInout, JoinMeta };
enum class TypeOperation { None,
Join,
JoinInout,
JoinMeta,
JoinNonexistent
};

static TypeOperation getTypeOperation(UnresolvedDotExpr *UDE,
ASTContext &Context) {
Expand All @@ -2970,6 +2975,7 @@ namespace {
.Case("type_join", TypeOperation::Join)
.Case("type_join_inout", TypeOperation::JoinInout)
.Case("type_join_meta", TypeOperation::JoinMeta)
.Case("type_join_nonexistent", TypeOperation::JoinNonexistent)
.Default(TypeOperation::None);
}

Expand Down Expand Up @@ -3034,6 +3040,25 @@ namespace {

return *join;
}

case TypeOperation::JoinNonexistent: {
auto lhsMeta = CS.getType(lhs)->getAs<MetatypeType>();
auto rhsMeta = CS.getType(rhs)->getAs<MetatypeType>();
if (!lhsMeta || !rhsMeta)
llvm_unreachable("Unexpected argument types for Builtin.type_join_nonexistent!");

auto &ctx = lhsMeta->getASTContext();

auto join =
Type::join(lhsMeta->getInstanceType(), rhsMeta->getInstanceType());

// Verify that we could not compute a join.
if (join)
llvm_unreachable("Unexpected result from join - it should not have been computable!");

// The return value is unimportant.
return MetatypeType::get(ctx.TheAnyType)->getCanonicalType();
}
}
}
};
Expand Down
7 changes: 5 additions & 2 deletions lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
}

// Verify noescape parameter uses.
checkNoEscapeParameterUse(DRE, nullptr, OperandKind::None);
checkNoEscapeParameterUse(DRE, Parent.getAsExpr(), OperandKind::None);

// Verify warn_unqualified_access uses.
checkUnqualifiedAccessUse(DRE);
Expand Down Expand Up @@ -703,7 +703,8 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
// The only valid use of the noescape parameter is an immediate call,
// either as the callee or as an argument (in which case, the typechecker
// validates that the noescape bit didn't get stripped off), or as
// a special case, in the binding of a withoutActuallyEscaping block.
// a special case, e.g. in the binding of a withoutActuallyEscaping block
// or the argument of a type(of: ...).
if (parent) {
if (auto apply = dyn_cast<ApplyExpr>(parent)) {
if (isa<ParamDecl>(DRE->getDecl()) && useKind == OperandKind::Callee)
Expand All @@ -714,6 +715,8 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
return;
} else if (isa<MakeTemporarilyEscapableExpr>(parent)) {
return;
} else if (isa<DynamicTypeExpr>(parent)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test case for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost all of the new tests hit this.

return;
}
}

Expand Down
33 changes: 32 additions & 1 deletion test/Sema/type_join.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import Swift
class C {}
class D : C {}

public func expectEqualType<T>(_: T.Type, _: T.Type) {}
func expectEqualType<T>(_: T.Type, _: T.Type) {}
func commonSupertype<T>(_: T, _: T) -> T {}

expectEqualType(Builtin.type_join(Int.self, Int.self), Int.self)
expectEqualType(Builtin.type_join_meta(D.self, C.self), C.self)
Expand Down Expand Up @@ -33,6 +34,36 @@ expectEqualType(Builtin.type_join(D?.self, Any.self), Any?.self)
expectEqualType(Builtin.type_join(Any?.self, Any.self), Any?.self)
expectEqualType(Builtin.type_join(Any.self, Any?.self), Any?.self)

func joinFunctions(
_ escaping: @escaping () -> (),
_ nonescaping: () -> ()
) {
_ = commonSupertype(escaping, escaping)
_ = commonSupertype(nonescaping, escaping)
// expected-error@-1 {{converting non-escaping value to 'T' may allow it to escape}}
_ = commonSupertype(escaping, nonescaping)
// expected-error@-1 {{converting non-escaping value to 'T' may allow it to escape}}
let x: Int = 1
// FIXME: We emit these diagnostics here because we refuse to allow
// Any to be inferred for the generic type. That's pretty
// arbitrary.
_ = commonSupertype(escaping, x)
// expected-error@-1 {{cannot convert value of type 'Int' to expected argument type '() -> ()'}}
_ = commonSupertype(x, escaping)
// expected-error@-1 {{cannot convert value of type '() -> ()' to expected argument type 'Int'}}

let a: Any = 1
_ = commonSupertype(nonescaping, a)
// expected-error@-1 {{converting non-escaping value to 'Any' may allow it to escape}}
_ = commonSupertype(a, nonescaping)
// expected-error@-1 {{converting non-escaping value to 'Any' may allow it to escape}}
_ = commonSupertype(escaping, a)
_ = commonSupertype(a, escaping)

expectEqualType(Builtin.type_join(((C) -> C).self, ((C) -> D).self),
((C) -> C).self)
}

func rdar37241221(_ a: C?, _ b: D?) {
let c: C? = C()
let array_c_opt = [c]
Expand Down