Skip to content

Modifies "maybeRewrap" to focus more on the maybe.#1900

Merged
paulp merged 1 commit intoscala:masterfrom
paulp:pr/maybe-rewrap-less
Jan 25, 2013
Merged

Modifies "maybeRewrap" to focus more on the maybe.#1900
paulp merged 1 commit intoscala:masterfrom
paulp:pr/maybe-rewrap-less

Conversation

@paulp
Copy link
Contributor

@paulp paulp commented Jan 15, 2013

Existential types are rewrapped under a bunch of conditions
unless the operation performed on the underlying type returns
the same type by reference equality. That depends on a
foundation of predictability which doesn't exist. The upshot is
that existential types were rewrapped with abandon, even when
the type were identical.

This had both performance and correctness implications.
Note where the test case output changes like so:

-scala.collection.immutable.List[Any]
+scala.collection.immutable.List[<?>]

That's correctness.

Existential types are rewrapped under a bunch of conditions
unless the operation performed on the underlying type returns
the same type by reference equality. That depends on a
foundation of predictability which doesn't exist. The upshot is
that existential types were rewrapped with abandon, even when
the type were identical.

This had both performance and correctness implications.
Note where the test case output changes like so:

  -scala.collection.immutable.List[Any]
  +scala.collection.immutable.List[<?>]

That's correctness.
@paulp
Copy link
Contributor Author

paulp commented Jan 15, 2013

Review by @adriaanm

@scala-jenkins
Copy link

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1395/

@scala-jenkins
Copy link

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1395/

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2115/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Failed - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2115/

@paulp
Copy link
Contributor Author

paulp commented Jan 15, 2013

PLS REBUILD ALL

@scala-jenkins
Copy link

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1403/

@scala-jenkins
Copy link

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1408/

@scala-jenkins
Copy link

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1408/

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2131/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2131/

@paulp
Copy link
Contributor Author

paulp commented Jan 25, 2013

BUMP @adriaanm

@adriaanm
Copy link
Contributor

LGTM

sorry about the slippage-from-radar

paulp added a commit that referenced this pull request Jan 25, 2013
Modifies "maybeRewrap" to focus more on the maybe.
@paulp paulp merged commit 634245c into scala:master Jan 25, 2013
@paulp paulp deleted the pr/maybe-rewrap-less branch January 25, 2013 15:56
adriaanm added a commit to adriaanm/scala that referenced this pull request Dec 23, 2016
We'll just have to live with manifests not being
able to resolve the difference between `scala.List[_]` and `scala.List[Any]`
(dealiasing causes rewrapping causes existential extrapolation).
They are equivalent types after all (and manifests are deprecated).

A type tag will be more precise.

This commit reverts scala#1900, which caused this regression.
Recently, scala#5263 improved the situation a bit, but showed that
the original tweak was not a good idea (too many exceptions).
Also, I doubt using `=:=` will improve performance, even if it
reduces allocation a bit, as it's far more expensive than `eq`...
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.

3 participants