[SE-0155][WIP] Normalize Enum Case Representation #12397

CodaFi · 2017-10-12T03:23:21Z

⚠️ DO NOT MERGE ⚠️

SE-0155 Requirements:

Ban case Foo(), rewrite to case Foo(Void)
Labels in associated value clauses are part of the case name
Case name overloading on distinct full names
Default arguments in associated value clauses
Consistent pattern matching behavior (see below)

A number of cleanups fell out of this patch:

EnumElementDecls owning a parameter list simplifies both serialization and AST printing
SourceKit now reports EnumElementDecls as owning a parameter list instead of a tuple
Function types in enum cases are implicitly marked @escaping and actually participate in semantic checks

Some open questions:

I would like the role of labels in patterns clarified before I finish off the work in TypeCheckPattern
The SILVerifier really doesn't like when uncurried applies and EnumInsts interact

CodaFi · 2017-10-12T03:49:25Z

lib/SIL/SILVerifier.cpp

+      //   indirect case elet(locals: [(String, Expr)], body: Expr)
+      // }
+      //
+      // Expr.elet(locals: body:)([], someExpr)


@gottesmm Any thoughts here?

What is "an uncurried EnumInst payload sans argument labels"?

I.e., can you elaborate?

See the example in the comment. The old check does strict type equality (labels included) rather than element-by-element equality (labels excluded)

Can you show me what the SIL looks like?

(I can't really say anything about SIL verification without seeing the SIL).

If you had said that sometimes you pass the payload boxed to the enum, I would have understood you immediately.

I would say the check should be that it is either the payload type or a boxed payload type.

(i.e. we had a language issue = p).

CodaFi · 2017-10-12T03:50:18Z

include/swift/AST/DiagnosticsParse.def

+ERROR(enum_element_empty_arglist,none,
+      "empty enum element argument list must be spelled with 'Void'", ())
+WARNING(enum_element_empty_arglist_swift3,none,
+        "empty enum element argument list must be spelled with 'Void'", ())


This needs phrasing as a warning

CodaFi · 2017-10-12T03:51:52Z

test/SourceKit/DocSupport/doc_swift_module.swift.response

@@ -1529,7 +1529,7 @@ func genfoo<T1, T2>(x ix: T1, y iy: T2) where T1 : Prot, T2 : cake.C1, T1.Elemen
        key.entities: [
          {
            key.kind: source.lang.swift.decl.enumelement,
-            key.name: "case1",
+            key.name: "case1()",


@benlangmuir Any idea what is causing SourceKit to serialize a null parameter list like this?

SourceKit is just calling ValueDecl::getFullName. This test is loading the decl from a Swift module though, so maybe check that?

CodaFi · 2018-01-07T01:45:48Z

@slavapestov I'm going to try to make everything go through applyNormalCall once the tests pass.

CodaFi · 2018-01-07T04:01:28Z

@swift-ci please smoke test

DevAndArtist · 2018-02-27T07:51:30Z

@CodaFi Any progress to this PR? I'm really looking forward to this feature. :)

CodaFi · 2018-02-27T19:38:36Z

Got a pile of test failures with this patch. Turns out changing SILGen changes SILGen

    Swift(macosx-x86_64) :: IRGen/enum_resilience.swift
    Swift(macosx-x86_64) :: SILGen/argument_shuffle_swift3.swift
    Swift(macosx-x86_64) :: SILGen/default_arguments.swift
    Swift(macosx-x86_64) :: SILGen/enum.swift
    Swift(macosx-x86_64) :: SILGen/errors.swift
    Swift(macosx-x86_64) :: SILGen/indirect_enum.swift
    Swift(macosx-x86_64) :: SILGen/opaque_values_silgen.swift
    Swift(macosx-x86_64) :: SILGen/toplevel_errors.swift
    Swift(macosx-x86_64) :: SILGen/tuples.swift
    Swift(macosx-x86_64) :: SILOptimizer/access_marker_verify.swift
    Swift(macosx-x86_64) :: SILOptimizer/definite-init-wrongscope.swift

dduan · 2018-03-02T22:51:33Z

I've rebased this PR last weekend: https://github.com/dduan/swift/tree/the-pitter-pattern-of-tiny-feet

CodaFi · 2018-03-18T04:43:26Z

In theory this passes the tests now

@swift-ci please smoke test

This models, but does not plumb through, default arguments.

CodaFi · 2018-03-19T02:11:15Z

@slavapestov I think a combination of my removing the applyEnumElementConstructor path and your work on SILGen for nil is causing malformed code to be emitted after you apply this patch:

SILGen/objc_blocks_bridging.swift

SIL verification failed: instruction isn't dominated by its operand: Dominance->properlyDominates(valueI, I)
Verifying instruction:
%35 = partial_apply [callee_guaranteed] %34(%33) : $@convention(thin) (@NoEscape @callee_guaranteed (@in ()) -> @out ()) -> () // users: %60, %36
-> destroy_value %35 : $@callee_guaranteed () -> () // id: %60

CodaFi · 2018-03-22T05:15:20Z

Closed for #15418

CodaFi commented Oct 12, 2017

View reviewed changes

CodaFi force-pushed the the-pitter-pattern-of-tiny-feet branch from fd94ef2 to 59447ec Compare October 12, 2017 23:46

CodaFi force-pushed the the-pitter-pattern-of-tiny-feet branch 2 times, most recently from 3180456 to e1e61e6 Compare January 7, 2018 01:20

CodaFi force-pushed the the-pitter-pattern-of-tiny-feet branch from e1e61e6 to f3db926 Compare January 7, 2018 03:59

CodaFi force-pushed the the-pitter-pattern-of-tiny-feet branch from f3db926 to 85ce4fc Compare February 27, 2018 19:36

rudkx self-requested a review February 27, 2018 20:43

CodaFi force-pushed the the-pitter-pattern-of-tiny-feet branch from 85ce4fc to b0e9362 Compare March 18, 2018 04:41

CodaFi added 9 commits March 18, 2018 20:35

Add a parameter list to EnumElementDecl

8727f54

This models, but does not plumb through, default arguments.

Ban empty argument lists in enum element decls

04a1809

Allow overloading on the full name of enum cases

9108e34

Make function types in enum cases @escaping

0032a03

Diagnose ambiguities in pattern lookup

883bdda

Add an initialization context for default arguments

a811303

SILGen default argument generators for enum cases

f5dc508

Add integration tests

4994fef

Always emit enum cases as normal applies

2373191

CodaFi force-pushed the the-pitter-pattern-of-tiny-feet branch from b0e9362 to 2373191 Compare March 19, 2018 02:09

CodaFi closed this Mar 22, 2018

CodaFi deleted the the-pitter-pattern-of-tiny-feet branch December 22, 2019 02:59

CodaFi mentioned this pull request Oct 12, 2017

[SR-4691] SE-0155 Normalize Enum Case Representation #47268

Open

[SE-0155][WIP] Normalize Enum Case Representation #12397

[SE-0155][WIP] Normalize Enum Case Representation #12397

Uh oh!

Conversation

CodaFi commented Oct 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CodaFi Oct 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CodaFi commented Jan 7, 2018

Uh oh!

CodaFi commented Jan 7, 2018

Uh oh!

DevAndArtist commented Feb 27, 2018

Uh oh!

CodaFi commented Feb 27, 2018

Uh oh!

dduan commented Mar 2, 2018

Uh oh!

CodaFi commented Mar 18, 2018

Uh oh!

CodaFi commented Mar 19, 2018

Uh oh!

CodaFi commented Mar 22, 2018

Uh oh!

Uh oh!

CodaFi commented Oct 12, 2017 •

edited

Loading

CodaFi Oct 12, 2017 •

edited

Loading