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

GenICode#adapt has several questionable cases #7159

Closed
scabug opened this issue Feb 20, 2013 · 4 comments
Closed

GenICode#adapt has several questionable cases #7159

scabug opened this issue Feb 20, 2013 · 4 comments
Milestone

Comments

@scabug
Copy link

scabug commented Feb 20, 2013

In GenICode the definition of adapt has several questionable cases that do not seem reachable. Need to investigate and either comment why they are necessary and add tests, or remove them if they are dead code.

This defines conforms as either a subtype relation or the case when from is null and to is nothing. When would we have a null when we expect nothing?

      val conforms = (from <:< to) || (from == NullReference && to == NothingReference)

This is saying if we have a throwable and a non-throwable is expected then we should emit a cast? Why would we get here?

        case ThrowableReference if !(ThrowableClass.tpe <:< to.toType) => ctx.bb.emit(CHECK_CAST(to)) // downcast throwables

And why do we need this special case? It's saying that from conformed with to, but that to is a LONG and from was int sized but that shouldn't happen, because conforms is defined as the subtype relation <:< and INT isn't a subtype of LONG however, removing it breaks test/files/run/t107.scala

        case _                                                         =>
          // widen subrange types
          if (from.isIntSizedType && to == LONG)
            coerce(INT, LONG)
@scabug
Copy link
Author

scabug commented Feb 20, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7159?orig=1
Reporter: @JamesIry
Assignee: @JamesIry

@scabug
Copy link
Author

scabug commented Feb 25, 2013

@JamesIry said:
I think the first two are legacies from early iterations on bolting "Null" and "Nothing" into the JVM. If I whack them the don't break any tests.

The last one does break a test if removed. So I'll look into why we need it.

@scabug
Copy link
Author

scabug commented Feb 28, 2013

@gkossakowski said:
Since scala/scala#2177 is merged, should we close this?

@scabug
Copy link
Author

scabug commented Aug 28, 2013

@gkossakowski said:
James, can we close this ticket?

@scabug scabug closed this as completed Nov 18, 2013
@scabug scabug added this to the 2.11.0 milestone Apr 7, 2017
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

No branches or pull requests

1 participant