Skip to content

Commit dd1b157

Browse files
committed
Partially Revert #27862
When SE-110 was being implemented, we accidentally began to accept closure parameter declarations that had no associated parameter names, e.g. foo { ([Int]) in /**/ } This syntax has never been sanctioned by any version of Swift and should be banned. However, the change was made long enough ago and there are enough clients relying on this, that we cannot accept the source break at the moment. For now, add a bit to ParamDecl that marks a parameter as destructured, and back out setting the invalid bit on the type repr for these kinds of declarations. To prevent further spread of this syntax, stub in a warning that offers to insert an anonymous parameter. Resolves part of rdar://56673657 and improves QoI for errors like rdar://56911630
1 parent 1ea23d9 commit dd1b157

File tree

9 files changed

+45
-11
lines changed

9 files changed

+45
-11
lines changed

include/swift/AST/Decl.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5214,7 +5214,7 @@ enum class ParamSpecifier : uint8_t {
52145214

52155215
/// A function parameter declaration.
52165216
class ParamDecl : public VarDecl {
5217-
Identifier ArgumentName;
5217+
llvm::PointerIntPair<Identifier, 1, bool> ArgumentNameAndDestructured;
52185218
SourceLoc ParameterNameLoc;
52195219
SourceLoc ArgumentNameLoc;
52205220
SourceLoc SpecifierLoc;
@@ -5251,7 +5251,9 @@ class ParamDecl : public VarDecl {
52515251
static ParamDecl *cloneWithoutType(const ASTContext &Ctx, ParamDecl *PD);
52525252

52535253
/// Retrieve the argument (API) name for this function parameter.
5254-
Identifier getArgumentName() const { return ArgumentName; }
5254+
Identifier getArgumentName() const {
5255+
return ArgumentNameAndDestructured.getPointer();
5256+
}
52555257

52565258
/// Retrieve the parameter (local) name for this function parameter.
52575259
Identifier getParameterName() const { return getName(); }
@@ -5270,6 +5272,9 @@ class ParamDecl : public VarDecl {
52705272
TypeRepr *getTypeRepr() const { return TyRepr; }
52715273
void setTypeRepr(TypeRepr *repr) { TyRepr = repr; }
52725274

5275+
bool isDestructured() const { return ArgumentNameAndDestructured.getInt(); }
5276+
void setDestructured(bool repr) { ArgumentNameAndDestructured.setInt(repr); }
5277+
52735278
DefaultArgumentKind getDefaultArgumentKind() const {
52745279
return static_cast<DefaultArgumentKind>(Bits.ParamDecl.defaultArgumentKind);
52755280
}

include/swift/AST/DiagnosticsParse.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,8 @@ ERROR(parameter_operator_keyword_argument,none,
893893

894894
ERROR(parameter_unnamed,none,
895895
"unnamed parameters must be written with the empty name '_'", ())
896+
WARNING(parameter_unnamed_warn,none,
897+
"unnamed parameters must be written with the empty name '_'", ())
896898

897899
ERROR(parameter_curry_syntax_removed,none,
898900
"cannot have more than one parameter list", ())

include/swift/Parse/Parser.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,9 @@ class Parser {
12111211

12121212
/// True if we emitted a parse error about this parameter.
12131213
bool isInvalid = false;
1214+
1215+
/// True if this parameter is potentially destructuring a tuple argument.
1216+
bool isPotentiallyDestructured = false;
12141217
};
12151218

12161219
/// Describes the context in which the given parameter is being parsed.

lib/AST/Decl.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5914,7 +5914,8 @@ ParamDecl::ParamDecl(SourceLoc specifierLoc,
59145914
VarDecl::Introducer::Let,
59155915
/*IsCaptureList*/ false, parameterNameLoc, parameterName, dc,
59165916
StorageIsNotMutable),
5917-
ArgumentName(argumentName), ParameterNameLoc(parameterNameLoc),
5917+
ArgumentNameAndDestructured(argumentName, false),
5918+
ParameterNameLoc(parameterNameLoc),
59185919
ArgumentNameLoc(argumentNameLoc), SpecifierLoc(specifierLoc) {
59195920
Bits.ParamDecl.SpecifierComputed = false;
59205921
Bits.ParamDecl.defaultArgumentKind =

lib/Parse/ParseExpr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2657,7 +2657,7 @@ parseClosureSignatureIfPresent(SmallVectorImpl<CaptureListEntry> &captureList,
26572657
// possible and give a proper fix-it. See SE-0110 for more details.
26582658
auto isTupleDestructuring = [](ParamDecl *param) -> bool {
26592659
auto *typeRepr = param->getTypeRepr();
2660-
if (!(typeRepr && typeRepr->isInvalid()))
2660+
if (!(typeRepr && param->isDestructured()))
26612661
return false;
26622662
return !param->hasName() && isa<TupleTypeRepr>(typeRepr);
26632663
};

lib/Parse/ParsePattern.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,12 +363,18 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
363363
if (param.Type) {
364364
// Mark current parameter type as invalid so it is possible
365365
// to diagnose it as destructuring of the closure parameter list.
366-
param.Type->setInvalid();
367-
param.isInvalid = true;
366+
param.isPotentiallyDestructured = true;
368367
if (!isClosure) {
369368
// Unnamed parameters must be written as "_: Type".
370369
diagnose(typeStartLoc, diag::parameter_unnamed)
371370
.fixItInsert(typeStartLoc, "_: ");
371+
} else {
372+
// Unnamed parameters were accidentally possibly accepted after
373+
// SE-110 depending on the kind of declaration. We now need to
374+
// warn about the misuse of this syntax and offer to
375+
// fix it.
376+
diagnose(typeStartLoc, diag::parameter_unnamed_warn)
377+
.fixItInsert(typeStartLoc, "_: ");
372378
}
373379
}
374380
} else {
@@ -492,7 +498,13 @@ mapParsedParameters(Parser &parser,
492498
// If we diagnosed this parameter as a parse error, propagate to the decl.
493499
if (paramInfo.isInvalid)
494500
param->setInvalid();
495-
501+
502+
// If we need to diagnose this parameter as a destructuring, propagate that
503+
// to the decl.
504+
// FIXME: This is a terrible way to catch this.
505+
if (paramInfo.isPotentiallyDestructured)
506+
param->setDestructured(true);
507+
496508
// If a type was provided, create the type for the parameter.
497509
if (auto type = paramInfo.Type) {
498510
// If 'inout' was specified, turn the type into an in-out type.

test/Constraints/tuple_arguments.swift

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,6 +1470,9 @@ let _ = sr4745.enumerated().map { (count, element) in "\(count): \(element)" }
14701470
let sr4738 = (1, (2, 3))
14711471
[sr4738].map { (x, (y, z)) -> Int in x + y + z }
14721472
// expected-error@-1 {{closure tuple parameter does not support destructuring}} {{20-26=arg1}} {{38-38=let (y, z) = arg1; }}
1473+
// expected-warning@-2 {{unnamed parameters must be written with the empty name '_'}} {{20-20=_: }}
1474+
// expected-error@-3 {{use of undeclared type 'y'}}
1475+
// expected-error@-4 {{use of undeclared type 'z'}}
14731476

14741477
// rdar://problem/31892961
14751478
let r31892961_1 = [1: 1, 2: 2]
@@ -1478,6 +1481,9 @@ r31892961_1.forEach { (k, v) in print(k + v) }
14781481
let r31892961_2 = [1, 2, 3]
14791482
let _: [Int] = r31892961_2.enumerated().map { ((index, val)) in
14801483
// expected-error@-1 {{closure tuple parameter does not support destructuring}} {{48-60=arg0}} {{3-3=\n let (index, val) = arg0\n }}
1484+
// expected-warning@-2 {{unnamed parameters must be written with the empty name '_'}} {{48-48=_: }}
1485+
// expected-error@-3 {{use of undeclared type 'index'}}
1486+
// expected-error@-4 {{use of undeclared type 'val'}}
14811487
val + 1
14821488
}
14831489

@@ -1490,12 +1496,16 @@ let r31892961_4 = (1, 2)
14901496
_ = [r31892961_4].map { x, y in x + y }
14911497

14921498
let r31892961_5 = (x: 1, (y: 2, (w: 3, z: 4)))
1493-
[r31892961_5].map { (x: Int, (y: Int, (w: Int, z: Int))) in x + y }
1499+
[r31892961_5].map { (x: Int, (y: Int, (w: Int, z: Int))) in x + y } // expected-note {{'x' declared here}}
14941500
// expected-error@-1 {{closure tuple parameter does not support destructuring}} {{30-56=arg1}} {{61-61=let (y, (w, z)) = arg1; }}
1501+
// expected-warning@-2 {{unnamed parameters must be written with the empty name '_'}} {{30-30=_: }}
1502+
// expected-error@-3{{use of unresolved identifier 'y'; did you mean 'x'?}}
14951503

14961504
let r31892961_6 = (x: 1, (y: 2, z: 4))
1497-
[r31892961_6].map { (x: Int, (y: Int, z: Int)) in x + y }
1505+
[r31892961_6].map { (x: Int, (y: Int, z: Int)) in x + y } // expected-note {{'x' declared here}}
14981506
// expected-error@-1 {{closure tuple parameter does not support destructuring}} {{30-46=arg1}} {{51-51=let (y, z) = arg1; }}
1507+
// expected-warning@-2 {{unnamed parameters must be written with the empty name '_'}} {{30-30=_: }}
1508+
// expected-error@-3{{use of unresolved identifier 'y'; did you mean 'x'?}}
14991509

15001510
// rdar://problem/32214649 -- these regressed in Swift 4 mode
15011511
// with SE-0110 because of a problem in associated type inference

test/decl/func/functions.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,8 @@ func bogusDestructuring() {
192192
func registerCallback(_ callback: @escaping (Bar?) -> Void) {} // expected-note {{found this candidate}}
193193
}
194194

195-
Foo().registerCallback { ([Bar]) in } // expected-error {{'<<error type>>' is not convertible to '[Bar]'}}
196-
Foo().registerCallback { ([String: Bar]) in } // expected-error {{'<<error type>>' is not convertible to '[Bar]'}}
195+
Foo().registerCallback { ([Bar]) in } // expected-warning {{unnamed parameters must be written with the empty name '_'}} {{29-29=_: }}
196+
Foo().registerCallback { ([String: Bar]) in }// expected-warning {{unnamed parameters must be written with the empty name '_'}} {{29-29=_: }}
197197
Foo().registerCallback { (Bar?) in } // expected-error {{ambiguous use of 'registerCallback'}}
198198
// expected-error@-1 {{expected parameter name followed by ':'}}
199199
// expected-error@-2 {{expected ',' separator}}

validation-test/compiler_crashers_fixed/28723-unreachable-executed-at-swift-lib-sema-csdiag-cpp-4012.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

88
// rdar://56144412
9+
// XFAIL: asserts
910
// RUN: not %target-swift-frontend %s -emit-ir
1011
func t(UInt=__FUNCTION__
1112
func&t(

0 commit comments

Comments
 (0)