Skip to content

Commit 1beb755

Browse files
CodaFiCodaFi
committed
Push FunctionTypeRepr Input Invariants Up
Validation of the input side of FunctionTypeRepr was previously being done in Sema because of expression folding. If we instead push the invariant that the input TypeRepr should always be a TupleTypeRepr into the AST a number of nice cleanups fall out: - The SIL Parser no longer accepts Swift 2-style type declarations - Parse is more cleanly able to reject invalid FunctionTypeReprs - Clients of the AST can be assured the input type is always a TupleType so we can flush Swift 2 hacks
1 parent ba4949b commit 1beb755

29 files changed

+213
-179
lines changed

‎include/swift/AST/DiagnosticsCommon.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ ERROR(generic_signature_not_minimal,none,
9090
ERROR(attr_only_on_parameters, none,
9191
"'%0' may only be used on parameters", (StringRef))
9292

93+
ERROR(function_type_no_parens,none,
94+
"single argument function types require parentheses", ())
95+
9396
#ifndef DIAG_NO_UNDEF
9497
# if defined(DIAG)
9598
# undef DIAG

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3465,8 +3465,6 @@ ERROR(objc_invalid_with_generic_params,none,
34653465
ERROR(objc_convention_invalid,none,
34663466
"%0 is not representable in Objective-C, so it cannot be used"
34673467
" with '@convention(%1)'", (Type, StringRef))
3468-
ERROR(function_type_no_parens,none,
3469-
"single argument function types require parentheses", ())
34703468
WARNING(paren_void_probably_void,none,
34713469
"when calling this function in Swift 4 or later, you must pass a '()' tuple; did you mean for the input type to be '()'?", ())
34723470
NOTE(not_objc_empty_protocol_composition,none,

‎include/swift/AST/TypeRepr.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ namespace swift {
3434
class DeclContext;
3535
class GenericEnvironment;
3636
class IdentTypeRepr;
37+
class TupleTypeRepr;
3738
class TypeDecl;
3839

3940
enum class TypeReprKind : uint8_t {
@@ -467,21 +468,23 @@ inline IdentTypeRepr::ComponentRange IdentTypeRepr::getComponentRange() {
467468

468469
/// \brief A function type.
469470
/// \code
470-
/// Foo -> Bar
471+
/// (Foo) -> Bar
472+
/// (Foo, Bar) -> Baz
473+
/// (x: Foo, y: Bar) -> Baz
471474
/// \endcode
472475
class FunctionTypeRepr : public TypeRepr {
473476
// These three are only used in SIL mode, which is the only time
474477
// we can have polymorphic function values.
475478
GenericParamList *GenericParams;
476479
GenericEnvironment *GenericEnv;
477480

478-
TypeRepr *ArgsTy;
481+
TupleTypeRepr *ArgsTy;
479482
TypeRepr *RetTy;
480483
SourceLoc ArrowLoc;
481484
SourceLoc ThrowsLoc;
482485

483486
public:
484-
FunctionTypeRepr(GenericParamList *genericParams, TypeRepr *argsTy,
487+
FunctionTypeRepr(GenericParamList *genericParams, TupleTypeRepr *argsTy,
485488
SourceLoc throwsLoc, SourceLoc arrowLoc, TypeRepr *retTy)
486489
: TypeRepr(TypeReprKind::Function),
487490
GenericParams(genericParams),
@@ -498,7 +501,7 @@ class FunctionTypeRepr : public TypeRepr {
498501
GenericEnv = genericEnv;
499502
}
500503

501-
TypeRepr *getArgsTypeRepr() const { return ArgsTy; }
504+
TupleTypeRepr *getArgsTypeRepr() const { return ArgsTy; }
502505
TypeRepr *getResultTypeRepr() const { return RetTy; }
503506
bool throws() const { return ThrowsLoc.isValid(); }
504507

@@ -511,7 +514,7 @@ class FunctionTypeRepr : public TypeRepr {
511514
static bool classof(const FunctionTypeRepr *T) { return true; }
512515

513516
private:
514-
SourceLoc getStartLocImpl() const { return ArgsTy->getStartLoc(); }
517+
SourceLoc getStartLocImpl() const;
515518
SourceLoc getEndLocImpl() const { return RetTy->getEndLoc(); }
516519
SourceLoc getLocImpl() const { return ArrowLoc; }
517520

‎lib/AST/TypeRepr.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,12 @@ TypeRepr *CloneVisitor::visitCompoundIdentTypeRepr(CompoundIdentTypeRepr *T) {
157157
}
158158

159159
TypeRepr *CloneVisitor::visitFunctionTypeRepr(FunctionTypeRepr *T) {
160-
return new (Ctx) FunctionTypeRepr(/*FIXME: Clone?*/T->getGenericParams(),
161-
visit(T->getArgsTypeRepr()),
162-
T->getThrowsLoc(),
163-
T->getArrowLoc(),
164-
visit(T->getResultTypeRepr()));
160+
return new (Ctx) FunctionTypeRepr(
161+
/*FIXME: Clone?*/T->getGenericParams(),
162+
cast<TupleTypeRepr>(visit(T->getArgsTypeRepr())),
163+
T->getThrowsLoc(),
164+
T->getArrowLoc(),
165+
visit(T->getResultTypeRepr()));
165166
}
166167

167168
TypeRepr *CloneVisitor::visitArrayTypeRepr(ArrayTypeRepr *T) {
@@ -478,6 +479,10 @@ SILBoxTypeRepr *SILBoxTypeRepr::create(ASTContext &C,
478479
ArgLAngleLoc, GenericArgs, ArgRAngleLoc);
479480
}
480481

482+
SourceLoc FunctionTypeRepr::getStartLocImpl() const {
483+
return ArgsTy->getStartLoc();
484+
}
485+
481486
SourceLoc SILBoxTypeRepr::getStartLocImpl() const {
482487
if (GenericParams && GenericParams->getSourceRange().isValid())
483488
return GenericParams->getSourceRange().Start;

‎lib/Parse/ParseType.cpp

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,24 +44,6 @@ TypeRepr *Parser::applyAttributeToType(TypeRepr *ty,
4444

4545
// Apply 'inout' or '__shared' or '__owned'
4646
if (specifierLoc.isValid()) {
47-
if (auto *fnTR = dyn_cast<FunctionTypeRepr>(ty)) {
48-
// If the input to the function isn't parenthesized, apply the inout
49-
// to the first (only) parameter, as we would in Swift 2. (This
50-
// syntax is deprecated in Swift 3.)
51-
TypeRepr *argsTR = fnTR->getArgsTypeRepr();
52-
if (!isa<TupleTypeRepr>(argsTR)) {
53-
auto *newArgsTR =
54-
new (Context) InOutTypeRepr(argsTR, specifierLoc);
55-
auto *newTR =
56-
new (Context) FunctionTypeRepr(fnTR->getGenericParams(),
57-
newArgsTR,
58-
fnTR->getThrowsLoc(),
59-
fnTR->getArrowLoc(),
60-
fnTR->getResultTypeRepr());
61-
newTR->setGenericEnvironment(fnTR->getGenericEnvironment());
62-
return newTR;
63-
}
64-
}
6547
switch (specifier) {
6648
case VarDecl::Specifier::Owned:
6749
ty = new (Context) OwnedTypeRepr(ty, specifierLoc);
@@ -461,7 +443,33 @@ ParserResult<TypeRepr> Parser::parseType(Diag<> MessageID,
461443
}
462444
SyntaxContext->addSyntax(Builder.build());
463445
}
464-
tyR = new (Context) FunctionTypeRepr(generics, tyR, throwsLoc, arrowLoc,
446+
447+
TupleTypeRepr *argsTyR = nullptr;
448+
if (auto *TTArgs = dyn_cast<TupleTypeRepr>(tyR)) {
449+
argsTyR = TTArgs;
450+
} else {
451+
bool isVoid = false;
452+
if (const auto Void = dyn_cast<SimpleIdentTypeRepr>(tyR)) {
453+
if (Void->getIdentifier().str() == "Void") {
454+
isVoid = true;
455+
}
456+
}
457+
458+
if (isVoid) {
459+
diagnose(tyR->getStartLoc(), diag::function_type_no_parens)
460+
.fixItReplace(tyR->getStartLoc(), "()");
461+
argsTyR = TupleTypeRepr::createEmpty(Context, tyR->getSourceRange());
462+
} else {
463+
diagnose(tyR->getStartLoc(), diag::function_type_no_parens)
464+
.highlight(tyR->getSourceRange())
465+
.fixItInsert(tyR->getStartLoc(), "(")
466+
.fixItInsertAfter(tyR->getEndLoc(), ")");
467+
argsTyR = TupleTypeRepr::create(Context, {tyR},
468+
tyR->getSourceRange());
469+
}
470+
}
471+
472+
tyR = new (Context) FunctionTypeRepr(generics, argsTyR, throwsLoc, arrowLoc,
465473
SecondHalf.get());
466474
} else if (generics) {
467475
// Only function types may be generic.

‎lib/Sema/TypeCheckConstraints.cpp

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,6 +1474,53 @@ TypeExpr *PreCheckExpression::simplifyTypeExpr(Expr *E) {
14741474
if (auto *AE = dyn_cast<ArrowExpr>(E)) {
14751475
if (!AE->isFolded()) return nullptr;
14761476

1477+
auto diagnoseMissingParens = [](TypeChecker &TC, TypeRepr *tyR) {
1478+
bool isVoid = false;
1479+
if (const auto Void = dyn_cast<SimpleIdentTypeRepr>(tyR)) {
1480+
if (Void->getIdentifier().str() == "Void") {
1481+
isVoid = true;
1482+
}
1483+
}
1484+
1485+
if (isVoid) {
1486+
TC.diagnose(tyR->getStartLoc(), diag::function_type_no_parens)
1487+
.fixItReplace(tyR->getStartLoc(), "()");
1488+
} else {
1489+
TC.diagnose(tyR->getStartLoc(), diag::function_type_no_parens)
1490+
.highlight(tyR->getSourceRange())
1491+
.fixItInsert(tyR->getStartLoc(), "(")
1492+
.fixItInsertAfter(tyR->getEndLoc(), ")");
1493+
}
1494+
};
1495+
1496+
auto extractInputTypeRepr = [&](Expr *E) -> TupleTypeRepr * {
1497+
if (!E)
1498+
return nullptr;
1499+
if (auto *TyE = dyn_cast<TypeExpr>(E)) {
1500+
auto ArgRepr = TyE->getTypeRepr();
1501+
if (auto *TTyRepr = dyn_cast<TupleTypeRepr>(ArgRepr))
1502+
return TTyRepr;
1503+
diagnoseMissingParens(TC, ArgRepr);
1504+
return TupleTypeRepr::create(TC.Context, {ArgRepr},
1505+
ArgRepr->getSourceRange());
1506+
}
1507+
if (auto *TE = dyn_cast<TupleExpr>(E))
1508+
if (TE->getNumElements() == 0)
1509+
return TupleTypeRepr::createEmpty(TC.Context, TE->getSourceRange());
1510+
1511+
// When simplifying a type expr like "(P1 & P2) -> (P3 & P4) -> Int",
1512+
// it may have been folded at the same time; recursively simplify it.
1513+
if (auto ArgsTypeExpr = simplifyTypeExpr(E)) {
1514+
auto ArgRepr = ArgsTypeExpr->getTypeRepr();
1515+
if (auto *TTyRepr = dyn_cast<TupleTypeRepr>(ArgRepr))
1516+
return TTyRepr;
1517+
diagnoseMissingParens(TC, ArgRepr);
1518+
return TupleTypeRepr::create(TC.Context, {ArgRepr},
1519+
ArgRepr->getSourceRange());
1520+
}
1521+
return nullptr;
1522+
};
1523+
14771524
auto extractTypeRepr = [&](Expr *E) -> TypeRepr * {
14781525
if (!E)
14791526
return nullptr;
@@ -1490,12 +1537,14 @@ TypeExpr *PreCheckExpression::simplifyTypeExpr(Expr *E) {
14901537
return nullptr;
14911538
};
14921539

1493-
TypeRepr *ArgsTypeRepr = extractTypeRepr(AE->getArgsExpr());
1540+
TupleTypeRepr *ArgsTypeRepr = extractInputTypeRepr(AE->getArgsExpr());
14941541
if (!ArgsTypeRepr) {
14951542
TC.diagnose(AE->getArgsExpr()->getLoc(),
14961543
diag::expected_type_before_arrow);
1497-
ArgsTypeRepr =
1498-
new (TC.Context) ErrorTypeRepr(AE->getArgsExpr()->getSourceRange());
1544+
auto ArgRange = AE->getArgsExpr()->getSourceRange();
1545+
auto ErrRepr =
1546+
new (TC.Context) ErrorTypeRepr(ArgRange);
1547+
ArgsTypeRepr = TupleTypeRepr::create(TC.Context, {ErrRepr}, ArgRange);
14991548
}
15001549

15011550
TypeRepr *ResultTypeRepr = extractTypeRepr(AE->getResultExpr());

‎lib/Sema/TypeCheckType.cpp

Lines changed: 22 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,7 +2032,7 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
20322032
attrs.clearAttribute(TAK_autoclosure);
20332033
}
20342034

2035-
auto *FuncTyInput = dyn_cast<TupleTypeRepr>(fnRepr->getArgsTypeRepr());
2035+
auto *FuncTyInput = fnRepr->getArgsTypeRepr();
20362036
if ((!FuncTyInput || FuncTyInput->getNumElements() != 0)
20372037
&& attrs.has(TAK_autoclosure)) {
20382038
TC.diagnose(attrs.getLoc(TAK_autoclosure),
@@ -2206,43 +2206,18 @@ Type TypeResolver::resolveASTFunctionType(FunctionTypeRepr *repr,
22062206

22072207
// If this is a function type without parens around the parameter list,
22082208
// diagnose this and produce a fixit to add them.
2209-
if (!isa<TupleTypeRepr>(repr->getArgsTypeRepr()) &&
2210-
!repr->isWarnedAbout()) {
2211-
auto args = repr->getArgsTypeRepr();
2212-
2213-
bool isVoid = false;
2214-
if (const auto Void = dyn_cast<SimpleIdentTypeRepr>(args)) {
2215-
if (Void->getIdentifier().str() == "Void") {
2216-
isVoid = true;
2217-
}
2218-
}
2219-
if (isVoid) {
2220-
TC.diagnose(args->getStartLoc(), diag::function_type_no_parens)
2221-
.fixItReplace(args->getStartLoc(), "()");
2222-
} else {
2223-
TC.diagnose(args->getStartLoc(), diag::function_type_no_parens)
2224-
.highlight(args->getSourceRange())
2225-
.fixItInsert(args->getStartLoc(), "(")
2226-
.fixItInsertAfter(args->getEndLoc(), ")");
2227-
}
2228-
2229-
// Don't emit this warning three times when in generics.
2230-
repr->setWarned();
2231-
} else if (isa<TupleTypeRepr>(repr->getArgsTypeRepr()) &&
2232-
!repr->isWarnedAbout()) {
2209+
if (!repr->isWarnedAbout()) {
22332210
// If someone wrote (Void) -> () in Swift 3, they probably meant
22342211
// () -> (), but (Void) -> () is (()) -> () so emit a warning
22352212
// asking if they meant () -> ().
22362213
auto args = repr->getArgsTypeRepr();
2237-
if (const auto Tuple = dyn_cast<TupleTypeRepr>(args)) {
2238-
if (Tuple->getNumElements() == 1) {
2239-
if (const auto Void =
2240-
dyn_cast<SimpleIdentTypeRepr>(Tuple->getElementType(0))) {
2241-
if (Void->getIdentifier().str() == "Void") {
2242-
TC.diagnose(args->getStartLoc(), diag::paren_void_probably_void)
2243-
.fixItReplace(args->getSourceRange(), "()");
2244-
repr->setWarned();
2245-
}
2214+
if (args->getNumElements() == 1) {
2215+
if (const auto Void =
2216+
dyn_cast<SimpleIdentTypeRepr>(args->getElementType(0))) {
2217+
if (Void->getIdentifier().str() == "Void") {
2218+
TC.diagnose(args->getStartLoc(), diag::paren_void_probably_void)
2219+
.fixItReplace(args->getSourceRange(), "()");
2220+
repr->setWarned();
22462221
}
22472222
}
22482223
}
@@ -2390,29 +2365,20 @@ Type TypeResolver::resolveSILFunctionType(FunctionTypeRepr *repr,
23902365
&*resolveSILFunctionGenericParams);
23912366
}
23922367

2393-
if (auto tuple = dyn_cast<TupleTypeRepr>(repr->getArgsTypeRepr())) {
2394-
// SIL functions cannot be variadic.
2395-
if (tuple->hasEllipsis()) {
2396-
TC.diagnose(tuple->getEllipsisLoc(), diag::sil_function_ellipsis);
2397-
}
2398-
// SIL functions cannot have parameter names.
2399-
for (auto &element : tuple->getElements()) {
2400-
if (element.UnderscoreLoc.isValid())
2401-
TC.diagnose(element.UnderscoreLoc, diag::sil_function_input_label);
2402-
}
2403-
2404-
for (auto elt : tuple->getElements()) {
2405-
auto param = resolveSILParameter(elt.Type,
2406-
options | TypeResolutionFlags::ImmediateFunctionInput);
2407-
params.push_back(param);
2408-
if (!param.getType()) return nullptr;
2368+
auto argsTuple = repr->getArgsTypeRepr();
2369+
// SIL functions cannot be variadic.
2370+
if (argsTuple->hasEllipsis()) {
2371+
TC.diagnose(argsTuple->getEllipsisLoc(), diag::sil_function_ellipsis);
2372+
}
2373+
// SIL functions cannot have parameter names.
2374+
for (auto &element : argsTuple->getElements()) {
2375+
if (element.UnderscoreLoc.isValid())
2376+
TC.diagnose(element.UnderscoreLoc, diag::sil_function_input_label);
2377+
}
24092378

2410-
if (param.getType()->hasError())
2411-
hasError = true;
2412-
}
2413-
} else {
2414-
SILParameterInfo param = resolveSILParameter(repr->getArgsTypeRepr(),
2415-
options | TypeResolutionFlags::ImmediateFunctionInput);
2379+
for (auto elt : argsTuple->getElements()) {
2380+
auto param = resolveSILParameter(elt.Type,
2381+
options | TypeResolutionFlags::ImmediateFunctionInput);
24162382
params.push_back(param);
24172383
if (!param.getType()) return nullptr;
24182384

‎test/IDE/complete_crashes.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func flip<A, B, C>(_ f: A -> B -> C) -> B -> A -> C { }
140140
func rdar22688199() {
141141
let f = flip(curried)(#^RDAR_22688199^#
142142
}
143-
// FLIP_CURRIED: Pattern/CurrModule: ['(']{#Int#}, {#Int#}[')'][#(Int) -> ()#]
143+
// FLIP_CURRIED: Pattern/CurrModule: ['(']{#(Int, Int)#}[')'][#(Int) -> ()#]
144144

145145
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RDAR_22836263
146146
func rdar22836263() {

‎test/IRGen/bridge_object_arm64.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,15 @@ entry(%c : $C, %w : $Builtin.Word):
4141
// CHECK: br label %tagged-cont
4242
// CHECK: tagged-cont:
4343
// CHECK: phi [[C]] [ [[TAGGED_RESULT]], %tagged-pointer ], [ [[MASKED_RESULT]], %not-tagged-pointer ]
44-
sil @convert_from_bridge_object : $Builtin.BridgeObject -> (ObjC, Builtin.Word) {
44+
sil @convert_from_bridge_object : $(Builtin.BridgeObject) -> (ObjC, Builtin.Word) {
4545
entry(%b : $Builtin.BridgeObject):
4646
%c = bridge_object_to_ref %b : $Builtin.BridgeObject to $ObjC
4747
%w = bridge_object_to_word %b : $Builtin.BridgeObject to $Builtin.Word
4848
%t = tuple (%c : $ObjC, %w : $Builtin.Word)
4949
return %t : $(ObjC, Builtin.Word)
5050
}
5151

52-
sil @convert_from_native_bridge_object : $Builtin.BridgeObject -> C {
52+
sil @convert_from_native_bridge_object : $(Builtin.BridgeObject) -> C {
5353
entry(%b : $Builtin.BridgeObject):
5454
%c = bridge_object_to_ref %b : $Builtin.BridgeObject to $C
5555
return %c : $C

‎test/IRGen/bridge_object_armv7.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,15 @@ entry(%c : $C, %w : $Builtin.Word):
2828
// CHECK: [[BOBITS:%.*]] = ptrtoint [[BRIDGE:%swift.bridge\*]] %0 to i32
2929
// CHECK: [[MASKED_BITS:%.*]] = and i32 [[BOBITS]], -4
3030
// CHECK: inttoptr i32 [[MASKED_BITS]] to [[C:%objc_object\*]]
31-
sil @convert_from_bridge_object : $Builtin.BridgeObject -> (ObjC, Builtin.Word) {
31+
sil @convert_from_bridge_object : $(Builtin.BridgeObject) -> (ObjC, Builtin.Word) {
3232
entry(%b : $Builtin.BridgeObject):
3333
%c = bridge_object_to_ref %b : $Builtin.BridgeObject to $ObjC
3434
%w = bridge_object_to_word %b : $Builtin.BridgeObject to $Builtin.Word
3535
%t = tuple (%c : $ObjC, %w : $Builtin.Word)
3636
return %t : $(ObjC, Builtin.Word)
3737
}
3838

39-
sil @convert_from_native_bridge_object : $Builtin.BridgeObject -> C {
39+
sil @convert_from_native_bridge_object : $(Builtin.BridgeObject) -> C {
4040
entry(%b : $Builtin.BridgeObject):
4141
%c = bridge_object_to_ref %b : $Builtin.BridgeObject to $C
4242
return %c : $C

‎test/IRGen/bridge_object_x86_64.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ entry(%c : $C, %w : $Builtin.Word):
4848
// CHECK: tagged-cont:
4949
// CHECK: phi [[C]] [ [[TAGGED_RESULT]], %tagged-pointer ], [ [[MASKED_RESULT]], %not-tagged-pointer ]
5050

51-
sil @convert_from_bridge_object : $Builtin.BridgeObject -> (ObjC, Builtin.Word) {
51+
sil @convert_from_bridge_object : $(Builtin.BridgeObject) -> (ObjC, Builtin.Word) {
5252
entry(%b : $Builtin.BridgeObject):
5353
%c = bridge_object_to_ref %b : $Builtin.BridgeObject to $ObjC
5454
%w = bridge_object_to_word %b : $Builtin.BridgeObject to $Builtin.Word
@@ -61,7 +61,7 @@ entry(%b : $Builtin.BridgeObject):
6161
// CHECK: [[MASKED_BITS:%.*]] = and i64 [[BOBITS]], 72057594037927928
6262
// CHECK: [[RESULT:%.*]] = inttoptr i64 [[MASKED_BITS]] to [[C:%T13bridge_object1CC\*]]
6363
// CHECK: ret %T13bridge_object1CC* [[RESULT]]
64-
sil @convert_from_native_bridge_object : $Builtin.BridgeObject -> C {
64+
sil @convert_from_native_bridge_object : $(Builtin.BridgeObject) -> C {
6565
entry(%b : $Builtin.BridgeObject):
6666
%c = bridge_object_to_ref %b : $Builtin.BridgeObject to $C
6767
return %c : $C

0 commit comments

Comments
 (0)