Skip to content
This repository was archived by the owner on Dec 22, 2021. It is now read-only.

StrawmanTest.mainTest failing in Scala 2.13.0-M3 #308

Closed
SethTisue opened this issue Nov 30, 2017 · 12 comments
Closed

StrawmanTest.mainTest failing in Scala 2.13.0-M3 #308

SethTisue opened this issue Nov 30, 2017 · 12 comments

Comments

@SethTisue
Copy link
Member

[collection-strawman] [error] Test strawman.collection.test.StrawmanTest.mainTest failed: java.lang.AssertionError: assertion failed, took 0.472 sec
[collection-strawman] [error]     at scala.Predef$.assert(Predef.scala:204)
[collection-strawman] [error]     at strawman.collection.test.StrawmanTest.lazyListOps(Test.scala:466)
[collection-strawman] [error]     at strawman.collection.test.StrawmanTest.mainTest(Test.scala:576)
[collection-strawman] [error]     ...

details at https://scala-ci.typesafe.com/view/scala-2.13.x/job/scala-2.13.x-integrate-community-build/856/consoleFull

doesn't seem to be intermittent (failed in both runs last night)

@SethTisue
Copy link
Member Author

or, hmm, maybe it is intermittent, there is a run currently in progress where collection-strawman already went green: https://scala-ci.typesafe.com/view/scala-2.13.x/job/scala-2.13.x-integrate-community-build/857/consoleFull

we'll see how the next few runs go

@julienrf
Copy link
Contributor

Did you change something about the evaluation of by-name right-associative operators?

@julienrf
Copy link
Contributor

Yeah now I remember that one: scala/scala#5969

But the test should always fail with the new evaluation model.

@SethTisue
Copy link
Member Author

SethTisue commented Nov 30, 2017

I failed to notice earlier that run 857 was manually triggered by Miles against a different 2.13 build.

So yes, always failing.

@julienrf
Copy link
Contributor

julienrf commented Dec 7, 2017

Hey @SethTisue! What should we do about that? Is this blocking you?

@SethTisue
Copy link
Member Author

it's not blocking me.

I don't understand the issue, I don't know what you should do. but in general, this repo ought to compile, and tests pass, on whatever the latest 2.13 nightly is, since before long some descendant of that nightly will become 2.13.0-M3. (@szeiger?)

@julienrf
Copy link
Contributor

julienrf commented Dec 12, 2017

The issue is that we have a test that checks the semantics of an operation (#:: on Stream LazyList) and that this semantics has changed because the language has changed. I would be in favor of changing the test so that it takes the new language semantics into account. However, the fact that the semantics of #:: will have changed between 2.12 and 2.13 might be a problem for existing users, though I doubt someone relies on the fact that this operator is not really lazy.

@SethTisue
Copy link
Member Author

I would be in favor of changing the test so that it takes the new language semantics into account

👍

@julienrf julienrf added this to the 0.8.0 milestone Dec 22, 2017
@julienrf julienrf modified the milestones: 0.8.0, 0.9.0 Jan 3, 2018
@julienrf
Copy link
Contributor

@szeiger I tried to make #:: a proper method on LazyList (rather than an extension method), like you did in scala/scala#5969 but then I noticed that it was not anymore possible to define lazy lists via recursive lazy vals:

lazy val fibs: LazyList[Int] = 0 #:: 1 #:: fibs.zip(fibs.tail).map { case (prevPrev, prev) => prev + prevPrev }

We get an infinite recursion. Is that expected?

Note that on one hand, @odersky thinks that recursive lazy vals should not be supported (scala/scala3#1892 (comment)). On the other hand, being able to define the fibonacci sequence as shown above would be useful, in my opinion.

@SethTisue
Copy link
Member Author

here's an example recursive lazy val Stream in my own code:

  implicit class RichStream[T](private val s: Stream[T]) extends AnyVal {
    // if we put the lazy val at the top level, we can't extend AnyVal,
    // so we make it local, like this:
    def circular: Stream[T] = {
      lazy val result: Stream[T] = s #::: result
      result
    }
}

the use of lazy val gives a true cycle (infinite if traversed, but occupying constant space in memory). still a niche use case, I guess, and wouldn't be horrible with a local var :-\

@szeiger
Copy link
Contributor

szeiger commented Jan 17, 2018

The Fibonacci example desugars to:

lazy val fibs: LazyList[Int] =
  fibs.zip(fibs.tail).map { case (prevPrev, prev) => prev + prevPrev }.#::(1).#::(0)

Since the receiver of a method call cannot be lazy, this will never work. We still need an extension method. The improvement by the new desugaring is that 0 is no longer forced (and 1 if #:: is not a lazy extension method).

julienrf added a commit to julienrf/collection-strawman that referenced this issue Jan 17, 2018
SethTisue added a commit to SethTisue/community-build that referenced this issue Jan 24, 2018
@julienrf julienrf removed this from the 0.9.0 milestone Jan 25, 2018
@SethTisue SethTisue changed the title StrawmanTest.mainTest failing in Scala 2.13 community build StrawmanTest.mainTest failing in Scala 2.13.0-M3 Feb 2, 2018
julienrf added a commit to julienrf/collection-strawman that referenced this issue Feb 19, 2018
@julienrf julienrf added this to the 0.11.0 milestone Feb 20, 2018
@szeiger szeiger removed this from the 0.11.0 milestone Apr 19, 2018
@julienrf
Copy link
Contributor

Fixed by scala/scala#6509

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants