[swift-evolution] Revisiting SE-0110

Xiaodi Wu xiaodi.wu at gmail.com
Fri Jun 16 22:22:21 CDT 2017


Keep in mind that once the latest proposal about enum cases is implemented,
these will be at least notionally no longer tuple labels but rather a
sugared way of spelling part of the case name. The rules surrounding labels
during case matching have only just been revised and approved and have not
even yet been implemented. I don’t think it would be wise to fiddle with
them again.


On Fri, Jun 16, 2017 at 21:21 Paul Cantrell <cantrell at pobox.com> wrote:

> On Jun 16, 2017, at 5:23 PM, Mark Lacey <mark.lacey at apple.com> wrote:
>
>
> On Jun 16, 2017, at 2:09 PM, Paul Cantrell <cantrell at pobox.com> wrote:
>
>
> On Jun 16, 2017, at 3:43 PM, Mark Lacey <mark.lacey at apple.com> wrote:
>
>
> On Jun 16, 2017, at 1:21 PM, Mark Lacey <mark.lacey at apple.com> wrote:
>
>
> On Jun 16, 2017, at 11:13 AM, Paul Cantrell via swift-evolution <
> swift-evolution at swift.org> wrote:
>
>
> On Jun 15, 2017, at 7:17 PM, Xiaodi Wu via swift-evolution <
> swift-evolution at swift.org> wrote:
>
>
> On Thu, Jun 15, 2017 at 19:03 Víctor Pimentel <vpimentel at tuenti.com>
> wrote:
>
>> On 16 Jun 2017, at 01:55, Xiaodi Wu via swift-evolution <
>> swift-evolution at swift.org> wrote:
>>
>> On Thu, Jun 15, 2017 at 17:43 David Hart <david at hartbit.com> wrote:
>>
>>>
>>> By the way, I’m not attempting to deduce that nobody uses this feature
>>> by the fact I didn’t know about it. But I think it’s one interesting
>>> datapoint when comparing it to SE-0110.
>>>
>>
>>
>> SE-0110, **in retrospect**, has had impacts on a lot of users;
>> prospectively, it was thought to be a minor change, even after review and
>> acceptance.
>>
>> Keep in mind that this proposed change would also eliminate inline tuple
>> shuffle. For instance, the following code will cease to compile:
>>
>> let x = (a: 1.0, r: 0.5, g: 0.5, b: 0.5)
>> func f(color: (r: Double, g: Double, b: Double, a: Double)) {
>>   print(color)
>> }
>> f(color: x)
>>
>> It is an open question how frequently this is used. But like implicit
>> tuple destructuring, it currently Just Works(TM) and users may not realize
>> they’re making use of the feature until it’s gone.
>>
>>
>> It's much much less used, by looking at open source projects I doubt that
>> a significant portion of projects would have to change code because of this.
>>
>
> The reason that I’m urging caution is because, if I recall correctly, that
> is also what we said about SE-0110 on this list. Then, as now, we were
> discussing an issue with something left over from the Swift 1 model of
> tuples. Then, as now, we believed that the feature in question was rarely
> used. Then, as now, we believed that removing that feature would improve
> consistency in the language, better both for the compiler and for users.
> Then, as now, leaving it in was thought to prevent moving forward with
> other features that could improve Swift.
>
>
> Data:
>
> I hacked up a regexp that will catch most uses of labeled tuples in
> pattern matches, e.g. “let (foo: bar) = baz”. That’s what we’re talking
> about, right?
>
>
> That’s the obvious example that people find confusing.
>
> Less obvious places that labeled tuple patterns show up are ‘case let’ and
> ‘case’ (see below).
>
>
> Okay, I should have looked at your regex and read further. It looks like
> you were already trying to match these.
>
>
> I did walk the grammar for all occurrences of _pattern_.
>
> I’m only matching named tuple patterns that immediately follow one of the
> keywords which a pattern follows (for, case, let, var, and catch). As I
> mentioned, I’m not matching patterns that come later in comma-separated
> lists. I’m also not matching named tuples inside nested patterns, e.g. let
> ((a: b), (c: d)).
>
> But again, if even the most basic form of this construct is so rare, I
> doubt more robust matching would turn up that much more usage.
>
> I’m surprised you’re not seeing any uses of ‘case’ with labels.
>
>
> Me too. But I just verified that my pattern does match them.
>
>
> Are you sure? It doesn’t look like it’s going to match the example I gave
> due to the leading ‘.’ on the enum case.
>
>
> Ah! I should have read your original message more carefully. You’re quite
> right, I only was checking case statements for raw tuples like this:
>
>     case let (i: a, f: b):
>
> …and not for anything involving associated values. I hadn’t even
> considered that associated values would be affected by this, but looking at
> the grammar it seems they would indeed be.
>
> Another clumsy regex search, this time for patterns with tuple labels on
> associated values, turned up 111 results (one per ~3800 lines). Not super
> common, but certainly nothing to sneeze at. Here they are:
>
>     https://gist.github.com/pcantrell/d32cdb5f7db6d6626e45e80011163efb
>
> Looking through that gist, these usages mostly strike me as being just
> fine:
>
>     case .cover(from: .bottom):
>
>     case .reference(with: let ref):
>
>     case .update(tableName: let tableName, columnNames: _):
>
> I’d even say that removing the tuple labels would make things worse.
> Consider:
>
>     case .name(last: let firstName, first: _):  // mistake is clear
>     case .name(let firstName, _):               // mistake is buried
>
> In Chris’s original brain-bending example, the confusion is that there’s
> no “let” after the colon, so Int and Float look like types instead of
> variable names:
>
>     let (a : Int, b : Float) = foo()
>
> However, in the examples in the gist above, most of the patterns either
> (1) declare variables using a `let` after the colon:
>
>     case .reference(with: let ref):
>
> …or (2) don’t declare a variable at all:
>
>     case .string(format: .some(.uri)):
>
> What if we allowed labels on associated values, but required a `let` after
> the colon to bind a variable?
>
>     case let .a(b: c):  // disallowed
>     case .a(b: let c):  // OK
>
> Only 15 of those 111 run afoul of _that_ rule. Here they are:
>
>     https://gist.github.com/pcantrell/9f61045d7d7c8d18eeec8ebbef6cd8f8
>
> That’s one breakage every ~28000 lines, which seems much more acceptable.
> The drawback is that you can’t declare variables for a bunch of associated
> value en masse anymore; you need one let per value. (See line 2 in that
> gist.)
>
> You might want to try the patch I sent as it will definitely catch any
> tuple pattern that makes it to the verifier and does have labels.
>
>
> I’m not set up to build the compiler, unfortunately. One of these days.
>
> P
>
>
> Mark
>
>
> P
>
>
> Mark
>
> Fortunately we do not appear to allow shuffling in these cases. I’m not
> sure if the human disambiguation is easier here because of the context
> (‘case let’ and ‘case’), but I don’t recall seeing complain about these
> being confusing (having said that it’s entirely possible they are very
> confusing the first time someone sees them, in particular ‘cast let’ and
> the binding form of ‘case’.
>
> enum X {
>   case e(i: Int, f: Float)
> }
>
> let x = X.e(i: 7, f: 12)
>
> if case let X.e(i: hi, f: bye) = x {
>   print("(i: \(hi), f: \(bye))")
> }
>
> func test(_ x: X, _ a: Int, _ b: Float) {
>   switch x {
>   case .e(i: a, f: b):
>     print("match values")
>   case .e(i: let _, f: let _):
>     print("bind values")
>   default:
>     break
>   }
> }
>
> test(X.e(i: 1, f: 2), 1, 2)
> test(X.e(i: 1, f: 2), 3, 4)
>
>
>
> I ran that against all 55 projects in swift-source-compat-suite,
> comprising about over 400,000 lines of Swift code, and found … drumroll …
> exactly one match:
>
>
> neota (swift-source-compat-suite)$ find project_cache -name '*.swift'
> -print0 | xargs -0 pcregrep -M
> '(for|case|let|var|catch)\s+\([a-zA-Z0-9_]+\s*:'
> project_cache/RxSwift/RxExample/RxExample-iOSTests/TestScheduler+MarbleTests.swift:
>               let (time: _, events: events) = segments.reduce((time: 0,
> events: [RecordedEvent]())) { state, event in
>
>
> Caveats about this method:
>
> • My regexp won’t match second and third patterns in a comma-separated let
> or case, e.g.:
>
>    let a = b, (c: d) = e
>
> • It doesn’t match non-ascii identifiers.
>
> • This experiment only considers labeled tuples in pattern matches, what I
> took Chris’s original puzzler to be about. Label-based tuple shuffling is a
> separate question.
>
> Still, even if it’s undercounting slightly, one breakage in half a million
> lines of code should put to rest concerns about unexpected widespread
> impact.
>
> (Anything else I’m missing?)
>
> • • •
>
> Aside for those who know the tools out there: what would it take to run
> inspections like this against ASTs instead of using a regex? Could we
> instrument the compiler as Brent suggested?
>
>
> If you want to catch *all* of these cases then the patch below will do it
> by failing the AST verifier when it hits a pattern with labels. If you only
> want to find the plain let-binding versions of this and not the ‘case let’
> and ‘case’ ones, I’d suggest looking at the parser to see if there’s an
> easy place to instrument (I don’t know offhand).
>
> Mark
>
> diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp
> index b59a7ade23..ba4b2a245d 100644
> --- a/lib/AST/ASTVerifier.cpp
> +++ b/lib/AST/ASTVerifier.cpp
> @@ -2772,6 +2772,13 @@ public:
>      }
>
>      void verifyParsed(TuplePattern *TP) {
> +      for (auto &elt : TP->getElements()) {
> +        if (!elt.getLabel().empty()) {
> +          Out << "Labeled tuple patterns are offensive!\n";
> +          abort();
> +        }
> +      }
> +
>        PrettyStackTracePattern debugStack(Ctx, "verifying TuplePattern",
> TP);
>        verifyParsedBase(TP);
>      }
>
>
>
>
> Or can SourceKit / SourceKitten give a full AST? Or has anybody written a
> Swift parser in Swift?
>
> Cheers,
>
> Paul
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170617/0c0167fa/attachment-0001.html>


More information about the swift-evolution mailing list