Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autofold #448

Merged
merged 6 commits into from
Nov 13, 2017
Merged

Autofold #448

merged 6 commits into from
Nov 13, 2017

Conversation

nomisRev
Copy link
Member

@nomisRev nomisRev commented Nov 7, 2017

Code generation that implements extension fold functions for sealed classes. Given following snippet.

@autofold sealed class Maybe<A> {
    data class Just<A>(val value: A): Maybe<A>()
    object Empty: Maybe<kotlin.Nothing>()
}

It will generate following code

inline fun <A, B> Maybe<A>.fold(
        crossinline just: (Maybe.Just<A>) -> B,
        crossinline empty: (Maybe.Empty) -> B
): B = when (this) {
    is Maybe.Just -> `just`(this)
    is Maybe.Empty -> `empty`(this)
}

It does not unwrap Just to (A) -> B because if there would be another subtype data class Another<A>(val value: A): Maybe<A>() you would lose type safety over function parameters but on the other hand I might make the code gen smart enough to check that... Thoughts?

Copy link
Member

@pakoito pakoito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd change the name to @fold, as we don't have names like @autoderive. The name of the functions could use a prefix like ifLeft & ifRight.

As for the implementation, what happens when a child class has more generic parameters than the parent class? i.e. sealed class Wrap<A>; data class W1<A, B>(val b: B): Wrap<A>?

@codecov-io
Copy link

codecov-io commented Nov 7, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@185cf36). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #448   +/-   ##
=========================================
  Coverage          ?   35.62%           
  Complexity        ?      327           
=========================================
  Files             ?      182           
  Lines             ?     4968           
  Branches          ?      532           
=========================================
  Hits              ?     1770           
  Misses            ?     3049           
  Partials          ?      149
Impacted Files Coverage Δ Complexity Δ
...ssor/src/main/java/kategory/fold/AnnotationInfo.kt 0% <0%> (ø) 0 <0> (?)
...c/main/java/kategory/fold/AutoFoldFileGenerator.kt 0% <0%> (ø) 0 <0> (?)
...r/src/main/java/kategory/fold/AutoFoldProcessor.kt 0% <0%> (ø) 0 <0> (?)
...r/src/main/java/kategory/fold/AnnotatedAutoFold.kt 0% <0%> (ø) 0 <0> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 185cf36...345b7a4. Read the comment docs.

@nomisRev
Copy link
Member Author

nomisRev commented Nov 7, 2017

@pakoito @fold is conflicts with kategory.optics.Fold if you have both optics and core imported.

@nomisRev
Copy link
Member Author

nomisRev commented Nov 7, 2017

@pakoito well damn :D 2 type arguments expected. Use 'W1<*, *>' if you don't want to pass type arguments

inline fun <A, B> `kategory`.`optics`.`Wrap`<A>.fold(
        crossinline w1: (`kategory`.`optics`.`W1`<A, B>) -> B
): B = when (this) {
    is `kategory`.`optics`.`W1` -> `w1`(this)
}

@nomisRev
Copy link
Member Author

nomisRev commented Nov 9, 2017

@pakoito what would you suggest here? Currently code gen bails with following message, since it impossible to write a fold method in this situation.

autofold.wrap.kt: (6, 8): 2 type arguments expected. Use 'W1<*, *>' if you don't want to pass type arguments
autofold.wrap.kt: (6, 41): Type mismatch: inferred type is Wrap<A> but W1<A, B> was expected

@pakoito
Copy link
Member

pakoito commented Nov 9, 2017

@nomisRev Add <*, *> to the when clause, but IIRC that means that the caller can assume the type of B and get a runtime error.

@raulraja
Copy link
Member

raulraja commented Nov 9, 2017

We don't support derivation at the moment anywhere else for kinds of arity higher than 1 so I think this is good as is until we want to support those across the board which we can tackle at once.

@nomisRev
Copy link
Member Author

nomisRev commented Nov 9, 2017

@pakoito I wouldn't add something that will lead to unsafe code.

@pakoito
Copy link
Member

pakoito commented Nov 10, 2017

@raulraja Doesn't this cause the same kind of conflict PartialFunction had? That's what I mean with unsafe.

@pakoito
Copy link
Member

pakoito commented Nov 11, 2017

@nomisRev @raulraja My suggestion going forward: fail the processor for those cases where child classes have more generic parameters than the sealed class.

@nomisRev
Copy link
Member Author

nomisRev commented Nov 12, 2017

@pakoito it currently fails for all cases where generics cannot be figured out and not just

where child classes have more generic parameters than the sealed class.

So no runtime errors can occur from using the generated fold methods. Currently it fails with the message of the invalid code but I will update it proper compile time error message before we merge.

@raulraja
Copy link
Member

@pakoito I think #448 (comment) addressed your concern. Can we merge this?

Copy link
Member

@pakoito pakoito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Green with NIT: improve the error message.

@raulraja raulraja merged commit 6993464 into master Nov 13, 2017
@raulraja raulraja deleted the simon-autofold branch November 13, 2017 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants