Skip to content

Int convert to Double when unnecessary #1533

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

Closed
XuefengWu opened this issue Sep 22, 2016 · 6 comments
Closed

Int convert to Double when unnecessary #1533

XuefengWu opened this issue Sep 22, 2016 · 6 comments

Comments

@XuefengWu
Copy link

  def f1(l: Array[Int]): Array[Any] =
       l.map {x =>
        if(x == 1) x.toString
        else if (x == 2) x.toInt
        else  x.toDouble
      }  

scala> f1(Array(1,2,3)).foreach(v => println(s"$v :${v.getClass}"))
1 :class java.lang.String
2.0 :class java.lang.Double  //this shoule be Int
3.0 :class java.lang.Double

the compiled code box the second result type is Double but not Int.

if use pattern match, it works well in dotty

def f2(l: Array[Int]): Array[Any] =
      l.map{ x =>
        x match {
        case x if x == 1 => x.toString
        case x if x == 2 => x.toInt
        case _ => x.toDouble
      }    
   }    

scala> f2(Array(1,2,3)).foreach(v => println(s"$v :${v.getClass}"))
1 :class java.lang.String
2 :class java.lang.Integer
3.0 :class java.lang.Double

but this also doest not works well in scalac 2.11/2.12

@DarkDimius
Copy link
Contributor

This is a result of weak subtyping(see Scala spec section 3.5.3).
if version is evaluated as follows:

if(x == 1) x.toString
else ({
  if (x == 2) x.toInt
  else  x.toDouble
}: Double)

where the second if part will be promoted to a common type that can capture both ints and doubles.

In case of pattern matching, all 3 versions are joined together as whole and there's no moment when compiler tries to maintain primitives.

@XuefengWu
Copy link
Author

Thank you for explain.
So only the scalac has bug for pattern match result type?

@DarkDimius
Copy link
Contributor

@XuefengWu. Pattern matching is underspecified in this regard. If you'd count scalac as reference implementation, our behaviour should be considered a bug instead.

Spec'ing pattern matching is something that we've been planning to do.

For those interested to see the underlying reason in discrepancy between Dotty and scalac:

  • scalac maintains a typer during all the phases in the tree generated by patmat actually looks quite similar to if version. This version gets typechecked and type adapations are inserted.
  • In dotty, we don't maintain typer, all the phases are required to generate well-typed trees and can't generate trees that-would-later-be-fixed by typer. This means that functionality normally available only in typer(weak subtyping adaptations) does not kick-in on tree generated by pattern matcher.

@XuefengWu
Copy link
Author

Thanks, I would open an issue for scalac.

@smarter
Copy link
Member

smarter commented Sep 22, 2016

Reopening so that we can discuss whether we would like to actually change this or not, maybe weak subtyping adaptations should not kick in when there's an expected type?

@odersky, what do you think of this:

  • scalac:
scala> (if (1 == 1) 1 else 2.0): Any
res0: Any = 1
  • dotty:
scala> (if (1 == 1) 1 else 2.0): Any
res0: Any = 1.0

@smarter smarter reopened this Sep 22, 2016
@odersky
Copy link
Contributor

odersky commented Jan 26, 2018

The original issue is fixed by now with the changes to harmonization. The code

(if (1 == 1) 1 else 2.0): Any

Still gives 1.0, but that's due to constant folding under harmronization. At present I don't feel it's worthwhile to stick a finger into that hornet's nest again. But if someone wants to revisit (including taking responsibility to push this through) feel free to revisit.

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

No branches or pull requests

4 participants