SI-8394 Add zip method to Option [jvican] #4836
Conversation
|
/rebuild |
|
I will try to figure out where the test failure is coming from. |
|
I think the failure is probably real (I am able to reproduce it locally). it could be some unexpected consequence of this change not being source compatible. in Scala 2.11 we have: scala> Some(1).zip(Some("foo"))
res0: Iterable[(Int, String)] = List((1,foo))but after this change we have: scala> Some(1).zip(Some("foo"))
res0: Option[(Int, String)] = Some((1,foo))perhaps there is existing code in the compiler/library somewhere which this breaks, which then shows up in the scalap failure. we might have to rethink whether we can accept this change for 2.12 given the 2.12 promise of "full backward source compatibility" with 2.11. and even for 2.13 it could be questionable. @retronym observes: "We had the same issue on 2.10 when we added something similar to option (flatten, I think). It was a big PITA to fix up in the code base I was working on. Small changes after the fact to widely used datatypes tend to be costly" |
|
I did try to see if the existing code in the Scala repo was using @retronym I'm curious if you think there is any other possible way that this change could be resulting in the scalap error we're getting |
|
Please rebase this on the tip of the 2.12.x branch. I believe the test failure will go away then. (The test was removed in 8d2d3c7) |
|
I have rebased on top of the last commit on the 2.12.x branch. Is that okay? |
|
I'm curious. What was the error that caused that weird behavior? |
|
on the question of the desirability of the change for 2.12: I'll bring it up at the next Scala team meeting. |
|
we discussed this and decided that it doesn't make sense for us to accept this in 2.12. it changes the meaning of existing code, in a release where we've already committed not to do that unless really necessary, and the benefit, while real, isn't that great. the change might still have a chance in 2.13. its chances would be improved if either or both of these come true:
so, I am sorry! I should have realized earlier in the review process that this change was problematic — that could have spared @kubudi and @jvican some work. |
Reduce the IMain interface used by the shell. Should capture this in an actual trait. So long, IMainOps
Carving out an interface that could cross process boundaries. (Strings/Trees/Positions, but no types/symbols) Similarly, model the subset of the repl used by sbt as `ReplCore`. For sbt/zinc 1.0, we should really sit down and agree on a new interface.
Parse user input as-is, and use that as content of unit's source file content. The unit's body has the trees resulting from wrapping and rewriting the user's input to implement the repl's semantics (store the result in a val etc). Also pass on whether parsed trees had xml elements. This allows us to be more precise in our error messages, as positions map 1-1 to user input (as suggested by retronym, who helped getting the range positions right). Note: input may contain leading/trailing non-trees. For example, whitespace/comments don't survive as trees, thus we can't assume the trees cover the range pos (0, buf.length); instead, compute it. Goal: no string-based code manipulation. No there yet. Handle assignment minimally to avoid memory leaks So, don't hang on to assigned values. Still provide some feedback that the assignment has been executed --> `"mutated x"`. Also, bring back pure expression warning to maintain status quo -- but who wants to see those pure expr warnings in the repl?? Rangepos invariants require most synthetic code to have a transparent pos, but the presentation compiler locates trees by position, which means use code must have an opaque position, and we must be able to navigate to it from the root, which means all ancestor trees to user code must cover the positions of said user code.
Move it there, and while we're at it, simplify Completion hierarchy
Invalidate symbols for artifact classfiles, refactor classfile parser
REPL: decouple the UI (shell) from the compiler [ci: last-only]
Document stability of sortBy
re-STARR on 2.13.0-M3
drop -Yuse-stupid-types
The bounds propagation introduced in scala#6140 caused a regression in scala/scala-java8-compat#97 because bounds sharpened while ranking implicit candidates leaked from candidates tested earlier to those tested later. This commit fixes that by saving and restoring the infos of the symbols of the undetermined type parameters around the call to type check the implicit candidate which tests for it's applicability. Initially it seemed like this ought to be a job for undoLog or Context#savingUndeteriminedTypeParams, but neither of those do the right thing here. UndoLog doesn't help because it affects the constraint on the corresponding TypeVar rather than the info of the symbol; and savingUndeterminedTypeParams doesn't help because the relevant call to typedImplicit shares the enclosing ImplicitSearch#undetParams rather than looking at the context.
Allow users to pattern match on instances of `Tuple2` with the alternative syntax `key -> value`, in line with the existing alternative syntax for constructing a `Tuple2`.
Removed -Yliteral-types option and cleaned up docs
…ing val. ...because rangepos expects offset-positioned trees not to contain range-positioned trees. This is the same approach that default arguments use when moving the RHS of the parameter val into a default getter. Alternatives: - Remove that assertion: I'm not sure what it's for, but the validation seems quite insistent on that point. I suspect that the IDE or PC or something using positions may break on that. - Use the tree duplicator (and don't make a focuser traverser). This is how the default args are currently done, but since we're not reusing `qual` here that I can see it doesn't seem too useful to waste the allocations that it would take to duplicate that tree. The test cases are just slapping `-Yrangepos` on all of the test cases introduced in bcbe993 (where this feature was introduced). `test/files/pos/t4225.scala` is not thus enhanced, since there's a similar rangeposly error with by-name arguments of right-associative methods. Fixes scala/bug#10706.
Add `->` alias for `Tuple2` to Predef
Make the production for `SimpleExpr1` match with the ones from the appendix.
Fix typo/dead code in grammar production in SLS
the hope of course is to get scala-xml out of the 2.13 bootstrap, but I'm not sure we'll get to it before M4. in the meantime, let's dogfood the new release.
Change "ns_per_<unit>" names in Duration.scala
upgrade scala-xml to 1.1.0
Tighten up check for List() to Nil rewrite
Deeply focus the qualifier before using it as the RHS of the stabilizing val.
Conflicts:
src/compiler/scala/tools/nsc/backend/jvm/CodeGen.scala
=> bean info removal vs backend parallel refactor
src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
=> parseExceptions moves vs addParamNames addition
src/intellij/scala.ipr.SAMPLE
=> SBT 0.13.17 upgraded, this section of the file is replaced by `sbt intellij` anyway.
src/library/mima-filters/2.12.0.backwards.excludes
=> no MiMa on this branch
src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
=> the usual
versions.properties
=> version bumps on 2.12 subsumed by already bumped versions on this branch.
…180301 Merge 2.12.x to 2.13.x [ci: last-only]
Merge 2.12 again
Don't leak sharpened bounds between implicit candidates
|
Will try to rebase it successfully in the future. Putting this on the backburner for now. |
So this is the active pull request of #4437.
Review by @SethTisue.