> On Jun 16, 2017, at 5:23 PM, Mark Lacey <[email protected]> wrote:
>
>
>> On Jun 16, 2017, at 2:09 PM, Paul Cantrell <[email protected]
>> <mailto:[email protected]>> wrote:
>>
>>>
>>> On Jun 16, 2017, at 3:43 PM, Mark Lacey <[email protected]
>>> <mailto:[email protected]>> wrote:
>>>
>>>
>>>> On Jun 16, 2017, at 1:21 PM, Mark Lacey <[email protected]
>>>> <mailto:[email protected]>> wrote:
>>>>
>>>>>
>>>>> On Jun 16, 2017, at 11:13 AM, Paul Cantrell via swift-evolution
>>>>> <[email protected] <mailto:[email protected]>> wrote:
>>>>>
>>>>>>
>>>>>> On Jun 15, 2017, at 7:17 PM, Xiaodi Wu via swift-evolution
>>>>>> <[email protected] <mailto:[email protected]>> wrote:
>>>>>>
>>>>>>
>>>>>> On Thu, Jun 15, 2017 at 19:03 Víctor Pimentel <[email protected]
>>>>>> <mailto:[email protected]>> wrote:
>>>>>> On 16 Jun 2017, at 01:55, Xiaodi Wu via swift-evolution
>>>>>> <[email protected] <mailto:[email protected]>> wrote:
>>>>>>
>>>>>>> On Thu, Jun 15, 2017 at 17:43 David Hart <[email protected]
>>>>>>> <mailto:[email protected]>> 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
<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
<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
>>>>> [email protected] <mailto:[email protected]>
>>>>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>>>> <https://lists.swift.org/mailman/listinfo/swift-evolution>
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution
