Skip to content

Conversation

johanandren
Copy link
Contributor

Fixes #24368

A bit on the fence about this, not source compatible and quite invasive.

However, the current situation is that methods that looks like they have a bound actually do not provide any type safety at all, for example this compiles fine and blows up later:

Source<Apple, NotUsed> apples = ???
Source<Dogs, NotUsed> dogs = ???
Source<Dogs, NotUsed> wat = apples.prepend(dogs)

@akka-ci akka-ci added validating PR is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR is currently being validated by Jenkins labels Feb 20, 2018
@akka-ci
Copy link

akka-ci commented Feb 20, 2018

Test FAILed.

@akka-ci akka-ci added validating PR is currently being validated by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) labels Feb 20, 2018
Copy link
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

LGTM, I guess

*
* {{{
* Source<Apple, NotUsed> apples = null;
* Source<Orange, NotUsed> oranges = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps use Source.empty in such examples instead of null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ups, yes, indeed, copy pasta from the test.

@@ -14,7 +14,8 @@ final class BidiFlow[-I1, +O1, -I2, +O2, +Mat](
override val shape: BidiShape[I1, O1, I2, O2]
) extends Graph[BidiShape[I1, O1, I2, O2], Mat] {

def asJava: javadsl.BidiFlow[I1, O1, I2, O2, Mat] = new javadsl.BidiFlow(this)
def asJava[JI1 <: I1, JO1 >: O1, JI2 <: I2, JO2 >: O2, JMat >: Mat]: javadsl.BidiFlow[JI1, JO1, JI2, JO2, JMat] =
Copy link
Contributor

Choose a reason for hiding this comment

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

that's some signature

Copy link
Contributor Author

Choose a reason for hiding this comment

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

;(

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed validating PR is currently being validated by Jenkins labels Feb 20, 2018
@akka-ci
Copy link

akka-ci commented Feb 20, 2018

Test PASSed.

@johanandren johanandren changed the title [WIP] java flow dsl lower bounds Java flow dsl lower bounds Feb 20, 2018
@akka-ci akka-ci added the tested PR that was successfully built and tested by Jenkins label Feb 20, 2018
@akka-ci
Copy link

akka-ci commented Feb 20, 2018

Test PASSed.

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed tested PR that was successfully built and tested by Jenkins validating PR is currently being validated by Jenkins labels Feb 21, 2018
@akka-ci
Copy link

akka-ci commented Feb 21, 2018

Test PASSed.

Copy link
Contributor

@jrudolph jrudolph left a comment

Choose a reason for hiding this comment

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

IMO the change is the right way to go, the question is under which conditions we can pull it off.

I also tried things back when @johanandren was working on it to find out whether there are better solutions but it seems that Java doesn't offer anything better. I guess the main reason why there is no better support in Java is that most collections are mutable and therefore must be invariant so the support for covariant data types is just too limited.

I wonder though, if this isn't a bug in the Scala compiler as well because it creates unsafe generic signatures. Shouldn't it create generic signatures that are just not usable from Java instead of bailing out and silently allowing unsafe usage?

@jrudolph
Copy link
Contributor

jrudolph commented Mar 5, 2018

Needs, rebasing/merging btw.

@johanandren johanandren force-pushed the wip-24368-java-flow-dsl-lower-bounds-johanandren branch from c70ec44 to 2ef76a6 Compare March 9, 2018 15:31
@akka-ci akka-ci added validating PR is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed tested PR that was successfully built and tested by Jenkins validating PR is currently being validated by Jenkins labels Mar 9, 2018
@akka-ci
Copy link

akka-ci commented Mar 9, 2018

Test FAILed.

@johanandren johanandren force-pushed the wip-24368-java-flow-dsl-lower-bounds-johanandren branch from 2ef76a6 to c09e274 Compare March 12, 2018 11:16
@akka-ci akka-ci added the validating PR is currently being validated by Jenkins label Mar 12, 2018
@akka-ci akka-ci added tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels Mar 12, 2018
@akka-ci
Copy link

akka-ci commented Mar 12, 2018

Test PASSed.

Copy link
Contributor

@2m 2m left a comment

Choose a reason for hiding this comment

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

LGTM. I see how removing these lower type bounds also required removing co/contra ascriptions from the type parameters in Source/Flow/Sink/etc.

Also needs a rebase.

@johanandren johanandren force-pushed the wip-24368-java-flow-dsl-lower-bounds-johanandren branch from c09e274 to 342ff0a Compare March 15, 2018 12:57
@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed tested PR that was successfully built and tested by Jenkins validating PR is currently being validated by Jenkins labels Mar 15, 2018
@akka-ci
Copy link

akka-ci commented Mar 15, 2018

Test PASSed.

@johanandren johanandren merged commit d8b9bb1 into akka:master Mar 15, 2018
@johanandren johanandren deleted the wip-24368-java-flow-dsl-lower-bounds-johanandren branch March 15, 2018 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants