Skip to content

Commit ff92d93

Browse files
hamishknighthamishknight
authored
Merge pull request #39193 from hamishknight/less-toil-more-tuple
[AST] Enforce that TupleExprs and ParenExprs dont have param flags
2 parents e13fafc + 2e82367 commit ff92d93

File tree

13 files changed

+116
-37
lines changed

13 files changed

+116
-37
lines changed

‎include/swift/AST/Types.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3005,12 +3005,11 @@ class AnyFunctionType : public TypeBase {
30053005

30063006
public:
30073007
/// Take an array of parameters and turn it into a tuple or paren type.
3008+
///
3009+
/// \param wantParamFlags Whether to preserve the parameter flags from the
3010+
/// given set of parameters.
30083011
static Type composeTuple(ASTContext &ctx, ArrayRef<Param> params,
3009-
bool canonicalVararg);
3010-
static Type composeTuple(ASTContext &ctx, CanParamArrayRef params,
3011-
bool canonicalVararg) {
3012-
return composeTuple(ctx, params.getOriginalArray(), canonicalVararg);
3013-
}
3012+
bool wantParamFlags = true);
30143013

30153014
/// Given two arrays of parameters determine if they are equal in their
30163015
/// canonicalized form. Internal labels and type sugar is *not* taken into

‎lib/APIDigester/ModuleAnalyzerNodes.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,8 +1519,7 @@ SwiftDeclCollector::constructTypeNode(Type T, TypeInitInfo Info) {
15191519
Root->addChild(constructTypeNode(Fun->getResult()));
15201520

15211521
auto Input = AnyFunctionType::composeTuple(Fun->getASTContext(),
1522-
Fun->getParams(),
1523-
/*canonicalVararg=*/false);
1522+
Fun->getParams());
15241523
Root->addChild(constructTypeNode(Input));
15251524
return Root;
15261525
}

‎lib/AST/ASTContext.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3457,12 +3457,12 @@ Type AnyFunctionType::Param::getParameterType(bool forCanonical,
34573457
}
34583458

34593459
Type AnyFunctionType::composeTuple(ASTContext &ctx, ArrayRef<Param> params,
3460-
bool canonicalVararg) {
3460+
bool wantParamFlags) {
34613461
SmallVector<TupleTypeElt, 4> elements;
34623462
for (const auto &param : params) {
3463-
Type eltType = param.getParameterType(canonicalVararg, &ctx);
3464-
elements.emplace_back(eltType, param.getLabel(),
3465-
param.getParameterFlags());
3463+
auto flags = wantParamFlags ? param.getParameterFlags()
3464+
: ParameterTypeFlags();
3465+
elements.emplace_back(param.getParameterType(), param.getLabel(), flags);
34663466
}
34673467
return TupleType::get(elements, ctx);
34683468
}

‎lib/AST/ASTVerifier.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,6 +1136,7 @@ class Verifier : public ASTWalker {
11361136
}
11371137

11381138
void verifyChecked(TupleExpr *E) {
1139+
PrettyStackTraceExpr debugStack(Ctx, "verifying TupleExpr", E);
11391140
const TupleType *exprTy = E->getType()->castTo<TupleType>();
11401141
for_each(exprTy->getElements().begin(), exprTy->getElements().end(),
11411142
E->getElements().begin(),
@@ -1148,8 +1149,14 @@ class Verifier : public ASTWalker {
11481149
Out << elt->getType() << "\n";
11491150
abort();
11501151
}
1152+
if (!field.getParameterFlags().isNone()) {
1153+
Out << "TupleExpr has non-empty parameter flags?\n";
1154+
Out << "sub expr: \n";
1155+
elt->dump(Out);
1156+
Out << "\n";
1157+
abort();
1158+
}
11511159
});
1152-
// FIXME: Check all the variadic elements.
11531160
verifyCheckedBase(E);
11541161
}
11551162

@@ -1970,10 +1977,15 @@ class Verifier : public ASTWalker {
19701977

19711978
void verifyChecked(ParenExpr *E) {
19721979
PrettyStackTraceExpr debugStack(Ctx, "verifying ParenExpr", E);
1973-
if (!isa<ParenType>(E->getType().getPointer())) {
1980+
auto ty = dyn_cast<ParenType>(E->getType().getPointer());
1981+
if (!ty) {
19741982
Out << "ParenExpr not of ParenType\n";
19751983
abort();
19761984
}
1985+
if (!ty->getParameterFlags().isNone()) {
1986+
Out << "ParenExpr has non-empty parameter flags?\n";
1987+
abort();
1988+
}
19771989
verifyCheckedBase(E);
19781990
}
19791991

‎lib/SILGen/SILGenApply.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4023,8 +4023,7 @@ RValue CallEmission::applyEnumElementConstructor(SGFContext C) {
40234023
std::move(*callSite).forward());
40244024

40254025
auto payloadTy = AnyFunctionType::composeTuple(SGF.getASTContext(),
4026-
resultFnType.getParams(),
4027-
/*canonicalVararg*/ true);
4026+
resultFnType->getParams());
40284027
auto arg = RValue(SGF, argVals, payloadTy->getCanonicalType());
40294028
payload = ArgumentSource(uncurriedLoc, std::move(arg));
40304029
formalResultType = cast<FunctionType>(formalResultType).getResult();

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2668,8 +2668,7 @@ loadIndexValuesForKeyPathComponent(SILGenFunction &SGF, SILLocation loc,
26682668

26692669
auto indexLoweredTy =
26702670
SGF.getLoweredType(
2671-
AnyFunctionType::composeTuple(SGF.getASTContext(), indexParams,
2672-
/*canonicalVararg=*/false));
2671+
AnyFunctionType::composeTuple(SGF.getASTContext(), indexParams));
26732672

26742673
auto addr = SGF.B.createPointerToAddress(loc, pointer,
26752674
indexLoweredTy.getAddressType(),

‎lib/Sema/CSSimplify.cpp

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2002,8 +2002,7 @@ static bool fixMissingArguments(ConstraintSystem &cs, ASTNode anchor,
20022002
// synthesized arguments to it.
20032003
if (argumentTuple) {
20042004
cs.addConstraint(ConstraintKind::Bind, *argumentTuple,
2005-
FunctionType::composeTuple(ctx, args,
2006-
/*canonicalVararg=*/false),
2005+
FunctionType::composeTuple(ctx, args),
20072006
cs.getConstraintLocator(anchor));
20082007
}
20092008

@@ -2206,21 +2205,39 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
22062205
// take multiple arguments to be passed as an argument in places
22072206
// that expect a function that takes a single tuple (of the same
22082207
// arity);
2209-
auto canImplodeParams = [&](ArrayRef<AnyFunctionType::Param> params) {
2208+
auto canImplodeParams = [&](ArrayRef<AnyFunctionType::Param> params,
2209+
const FunctionType *destFn) {
22102210
if (params.size() == 1)
22112211
return false;
22122212

2213-
for (auto param : params)
2214-
if (param.isVariadic() || param.isInOut() || param.isAutoClosure())
2215-
return false;
2213+
// We do not support imploding into a @differentiable function.
2214+
if (destFn->isDifferentiable())
2215+
return false;
22162216

2217+
for (auto &param : params) {
2218+
// We generally cannot handle parameter flags, though we can carve out an
2219+
// exception for ownership flags such as __owned, which we can thunk, and
2220+
// flags that can freely dropped from a function type such as
2221+
// @_nonEphemeral. Note that @noDerivative can also be freely dropped, as
2222+
// we've already ensured that the destination function is not
2223+
// @differentiable.
2224+
auto flags = param.getParameterFlags();
2225+
flags = flags.withValueOwnership(
2226+
param.isInOut() ? ValueOwnership::InOut : ValueOwnership::Default);
2227+
flags = flags.withNonEphemeral(false)
2228+
.withNoDerivative(false);
2229+
if (!flags.isNone())
2230+
return false;
2231+
}
22172232
return true;
22182233
};
22192234

22202235
auto implodeParams = [&](SmallVectorImpl<AnyFunctionType::Param> &params) {
2236+
// Form an imploded tuple type, dropping the parameter flags as although
2237+
// canImplodeParams makes sure we're not dealing with vargs, inout, etc,
2238+
// we may still have e.g ownership flags left over, which we can drop.
22212239
auto input = AnyFunctionType::composeTuple(getASTContext(), params,
2222-
/*canonicalVararg=*/false);
2223-
2240+
/*wantParamFlags*/ false);
22242241
params.clear();
22252242
// If fixes are disabled let's do an easy thing and implode
22262243
// tuple directly into parameters list.
@@ -2255,12 +2272,12 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
22552272
if (last != path.rend()) {
22562273
if (last->getKind() == ConstraintLocator::ApplyArgToParam) {
22572274
if (isSingleTupleParam(ctx, func2Params) &&
2258-
canImplodeParams(func1Params)) {
2275+
canImplodeParams(func1Params, /*destFn*/ func2)) {
22592276
implodeParams(func1Params);
22602277
increaseScore(SK_FunctionConversion);
22612278
} else if (!ctx.isSwiftVersionAtLeast(5) &&
22622279
isSingleTupleParam(ctx, func1Params) &&
2263-
canImplodeParams(func2Params)) {
2280+
canImplodeParams(func2Params, /*destFn*/ func1)) {
22642281
auto *simplified = locator.trySimplifyToExpr();
22652282
// We somehow let tuple unsplatting function conversions
22662283
// through in some cases in Swift 4, so let's let that
@@ -2296,11 +2313,11 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
22962313
// 2. `case .bar(let tuple) = e` allows to match multiple
22972314
// parameters with a single tuple argument.
22982315
if (isSingleTupleParam(ctx, func1Params) &&
2299-
canImplodeParams(func2Params)) {
2316+
canImplodeParams(func2Params, /*destFn*/ func1)) {
23002317
implodeParams(func2Params);
23012318
increaseScore(SK_FunctionConversion);
23022319
} else if (isSingleTupleParam(ctx, func2Params) &&
2303-
canImplodeParams(func1Params)) {
2320+
canImplodeParams(func1Params, /*destFn*/ func2)) {
23042321
implodeParams(func1Params);
23052322
increaseScore(SK_FunctionConversion);
23062323
}
@@ -2311,7 +2328,7 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
23112328
auto *anchor = locator.trySimplifyToExpr();
23122329
if (isa_and_nonnull<ClosureExpr>(anchor) &&
23132330
isSingleTupleParam(ctx, func2Params) &&
2314-
canImplodeParams(func1Params)) {
2331+
canImplodeParams(func1Params, /*destFn*/ func2)) {
23152332
auto *fix = AllowClosureParamDestructuring::create(
23162333
*this, func2, getConstraintLocator(anchor));
23172334
if (recordFix(fix))
@@ -6073,8 +6090,7 @@ ConstraintSystem::simplifyConstructionConstraint(
60736090

60746091
// Tuple construction is simply tuple conversion.
60756092
Type argType = AnyFunctionType::composeTuple(getASTContext(),
6076-
fnType->getParams(),
6077-
/*canonicalVararg=*/false);
6093+
fnType->getParams());
60786094
Type resultType = fnType->getResult();
60796095

60806096
ConstraintLocatorBuilder builder(locator);
@@ -7006,8 +7022,7 @@ ConstraintSystem::simplifyFunctionComponentConstraint(
70067022

70077023
if (kind == ConstraintKind::FunctionInput) {
70087024
type = AnyFunctionType::composeTuple(getASTContext(),
7009-
funcTy->getParams(),
7010-
/*canonicalVararg=*/false);
7025+
funcTy->getParams());
70117026
locKind = ConstraintLocator::FunctionArgument;
70127027
} else if (kind == ConstraintKind::FunctionResult) {
70137028
type = funcTy->getResult();

‎lib/Sema/TypeCheckDeclOverride.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ static bool noteFixableMismatchedTypes(ValueDecl *decl, const ValueDecl *base) {
482482
auto *fnType = baseTy->getAs<AnyFunctionType>();
483483
baseTy = fnType->getResult();
484484
Type argTy = FunctionType::composeTuple(
485-
ctx, baseTy->getAs<AnyFunctionType>()->getParams(), false);
485+
ctx, baseTy->getAs<AnyFunctionType>()->getParams());
486486
auto diagKind = diag::override_type_mismatch_with_fixits_init;
487487
unsigned numArgs = baseInit->getParameters()->size();
488488
return computeFixitsForOverridenDeclaration(

‎lib/Sema/TypeCheckProtocol.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2164,8 +2164,7 @@ static Type getTypeForDisplay(ModuleDecl *module, ValueDecl *decl) {
21642164
return AnyFunctionType::composeTuple(module->getASTContext(),
21652165
ctor->getMethodInterfaceType()
21662166
->castTo<FunctionType>()
2167-
->getParams(),
2168-
/*canonicalVararg=*/false);
2167+
->getParams());
21692168
}
21702169

21712170
Type type = decl->getInterfaceType();

‎test/AutoDiff/Sema/differentiable_attr_type_checking.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,3 +726,24 @@ struct Accessors: Differentiable {
726726
// expected-error @+1 {{cannot differentiate functions returning opaque result types}}
727727
@differentiable(reverse)
728728
func opaqueResult(_ x: Float) -> some Differentiable { x }
729+
730+
// Test the function tupling conversion with @differentiable.
731+
func tuplify<Ts, U>(_ fn: @escaping (Ts) -> U) -> (Ts) -> U { fn }
732+
func tuplifyDifferentiable<Ts : Differentiable, U>(_ fn: @escaping @differentiable(reverse) (Ts) -> U) -> @differentiable(reverse) (Ts) -> U { fn }
733+
734+
func testTupling(withoutNoDerivative: @escaping @differentiable(reverse) (Float, Float) -> Float,
735+
withNoDerivative: @escaping @differentiable(reverse) (Float, @noDerivative Float) -> Float) {
736+
// We support tupling of differentiable functions as long as they drop @differentiable.
737+
let _: ((Float, Float)) -> Float = tuplify(withoutNoDerivative)
738+
let fn1 = tuplify(withoutNoDerivative)
739+
_ = fn1((0, 0))
740+
741+
// In this case we also drop @noDerivative.
742+
let _: ((Float, Float)) -> Float = tuplify(withNoDerivative)
743+
let fn2 = tuplify(withNoDerivative)
744+
_ = fn2((0, 0))
745+
746+
// We do not support tupling into an @differentiable function.
747+
let _ = tuplifyDifferentiable(withoutNoDerivative) // expected-error {{cannot convert value of type '@differentiable(reverse) (Float, Float) -> Float' to expected argument type '@differentiable(reverse) (Float) -> Float'}}
748+
let _ = tuplifyDifferentiable(withNoDerivative) // expected-error {{cannot convert value of type '@differentiable(reverse) (Float, @noDerivative Float) -> Float' to expected argument type '@differentiable(reverse) (Float) -> Float'}}
749+
}

‎test/Concurrency/isolated_parameters.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,13 @@ actor TestActor {
102102

103103
func redecl(_: TestActor) { } // expected-note{{'redecl' previously declared here}}
104104
func redecl(_: isolated TestActor) { } // expected-error{{invalid redeclaration of 'redecl'}}
105+
106+
func tuplify<Ts>(_ fn: (Ts) -> Void) {} // expected-note {{in call to function 'tuplify'}}
107+
108+
@available(SwiftStdlib 5.5, *)
109+
func testTuplingIsolated(_ a: isolated A, _ b: isolated A) {
110+
// FIXME: We shouldn't duplicate the "cannot convert value of type" diagnostic (SR-15179)
111+
tuplify(testTuplingIsolated)
112+
// expected-error@-1 {{generic parameter 'Ts' could not be inferred}}
113+
// expected-error@-2 2{{cannot convert value of type '(isolated A, isolated A) -> ()' to expected argument type '(Ts) -> Void'}}
114+
}

‎test/Constraints/function.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,16 @@ func test_passing_noescape_function_ref_to_generic_parameter() {
250250
func SR14784<T>(_ fs: () -> T..., a _ : Int) -> T {
251251
fs.first! // expected-error{{function produces expected type 'T'; did you mean to call it with '()'?}} {{11-11=()}}
252252
}
253+
254+
func tuplify<Ts>(_ fn: (Ts) -> Void) {}
255+
256+
func testInvalidTupleImplosions() {
257+
func takesVargs(_ x: Int, _ y: String...) {}
258+
tuplify(takesVargs) // expected-error {{cannot convert value of type '(Int, String...) -> ()' to expected argument type '(Int) -> Void'}}
259+
260+
func takesAutoclosure(_ x: @autoclosure () -> Int, y: String) {}
261+
tuplify(takesAutoclosure) // expected-error {{cannot convert value of type '(@autoclosure () -> Int, String) -> ()' to expected argument type '(@escaping () -> Int) -> Void'}}
262+
263+
func takesInout(_ x: Int, _ y: inout String) {}
264+
tuplify(takesInout) // expected-error {{cannot convert value of type '(Int, inout String) -> ()' to expected argument type '(Int) -> Void'}}
265+
}

‎test/Sema/diag_non_ephemeral.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,3 +525,16 @@ func ambiguous_fn(_ ptr: UnsafeRawPointer) {} // expected-note {{found this cand
525525
func test_ambiguity_with_function_instead_of_argument(_ x: inout Int) {
526526
ambiguous_fn(&x) // expected-error {{ambiguous use of 'ambiguous_fn'}}
527527
}
528+
529+
func tuplify<Ts>(_ fn: @escaping (Ts) -> Void) -> (Ts) -> Void { fn }
530+
531+
func testTuplingNonEphemeral(_ ptr: UnsafePointer<Int>) {
532+
// Make sure we drop @_nonEphemeral when imploding params. This is to ensure
533+
// we don't accidently break any potentially valid code.
534+
let fn = tuplify(takesTwoPointers)
535+
fn((ptr, ptr))
536+
537+
// Note we can't perform X-to-pointer conversions in this case even if we
538+
// wanted to.
539+
fn(([1], ptr)) // expected-error {{tuple type '([Int], UnsafePointer<Int>)' is not convertible to tuple type '(UnsafePointer<Int>, UnsafePointer<Int>)'}}
540+
}

0 commit comments

Comments
 (0)