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

Deprecate +(x: String): String on primitives #6755

Merged
merged 1 commit into from
Jun 8, 2018

Conversation

smarter
Copy link
Member

@smarter smarter commented Jun 8, 2018

This is consistent with the deprecation of any2stringadd in #6315.

@scala-jenkins scala-jenkins added this to the 2.13.0-M5 milestone Jun 8, 2018
@smarter
Copy link
Member Author

smarter commented Jun 8, 2018

Somehow this broke specialization of Function:

> scalac test/files/run/Course-2002-02.scala
> scala Test
[...]
java.lang.NoSuchMethodError: scala.Function2.apply$mcDII$sp(II)D
        at M4$.<init>(Course-2002-02.scala:106)
        at M4$.<clinit>(Course-2002-02.scala)
        at Test$.main(Course-2002-02.scala:533)
        at Test.main(Course-2002-02.scala)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at scala.reflect.internal.util.ScalaClassLoader.$anonfun$run$2(ScalaClassLoader.scala:99)
        at scala.reflect.internal.util.ScalaClassLoader.asContext(ScalaClassLoader.scala:33)
        at scala.reflect.internal.util.ScalaClassLoader.asContext$(ScalaClassLoader.scala:31)
        at scala.reflect.internal.util.ScalaClassLoader$URLClassLoader.asContext(ScalaClassLoader.scala:130)
        at scala.reflect.internal.util.ScalaClassLoader.run(ScalaClassLoader.scala:99)
        at scala.reflect.internal.util.ScalaClassLoader.run$(ScalaClassLoader.scala:91)
        at scala.reflect.internal.util.ScalaClassLoader$URLClassLoader.run(ScalaClassLoader.scala:130)
        at scala.tools.nsc.CommonRunner.run(ObjectRunner.scala:22)
        at scala.tools.nsc.CommonRunner.run$(ObjectRunner.scala:21)
        at scala.tools.nsc.ObjectRunner$.run(ObjectRunner.scala:39)
        at scala.tools.nsc.CommonRunner.runAndCatch(ObjectRunner.scala:29)
        at scala.tools.nsc.CommonRunner.runAndCatch$(ObjectRunner.scala:28)
        at scala.tools.nsc.ObjectRunner$.runAndCatch(ObjectRunner.scala:39)
        at scala.tools.nsc.MainGenericRunner.runTarget$1(MainGenericRunner.scala:63)
        at scala.tools.nsc.MainGenericRunner.run$1(MainGenericRunner.scala:83)
        at scala.tools.nsc.MainGenericRunner.process(MainGenericRunner.scala:94)
        at scala.tools.nsc.MainGenericRunner$.main(MainGenericRunner.scala:99)
        at scala.tools.nsc.MainGenericRunner.main(MainGenericRunner.scala)

I don't have the faintest idea why. Any clue?

@smarter
Copy link
Member Author

smarter commented Jun 8, 2018

Somehow this broke specialization of Function

Looks like it works on the CI and also works locally after doing a clean ¯\_(ツ)_/¯

Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you!

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work.

@Jasper-M
Copy link
Contributor

Jasper-M commented Jun 8, 2018

Next up: changing + on String to only accept a String?

lrytz
lrytz previously requested changes Jun 8, 2018
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

println(test5(b = 1, a = 2)(3, "4", "4", "4"))
println(test5(b = 1, a = 2)(c = 29))


// tuple conversion
def foo(a: Int, b: Int)(c: (Int, String)) = a + c._1
def foo(a: Int, b: Int)(c: (Int, String)) = s"$a${c._1}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was actually an addition 🤣

This is consistent with the deprecation of any2stringadd in scala#6315.
@smarter smarter force-pushed the deprecate/number-plus-string branch from d71f702 to 15fd2c9 Compare June 8, 2018 13:58
@lrytz lrytz merged commit 5e9ff0d into scala:2.13.x Jun 8, 2018
@xuwei-k
Copy link
Contributor

xuwei-k commented Jul 16, 2018

scala/bug#11025

@nafg
Copy link
Contributor

nafg commented Aug 10, 2018

Should this have a release notes label like #6315 ?

@nafg
Copy link
Contributor

nafg commented Aug 10, 2018

I'm not sure I get the point of this. An overbroad method on Any is one thing. What are some pitfalls that removing it from primitives avoids?

@dwijnand dwijnand added the release-notes worth highlighting in next release notes label Aug 10, 2018
@Ichoran
Copy link
Contributor

Ichoran commented Aug 10, 2018

@nafg - It avoids strings gobbling up every other type when you didn't intend it. Normally this wouldn't be an issue because the type would be wrong and it'd be caught soon enough downstream, but if you end up wanting a string at the end anyway (because it's output), you can end up in a situation where you meant to do some math and report on the outcome, but instead you concatenated a number with something. (I have actually done this.)

It also reduces the number of confusing "Why does it say I can't have a String here?! I don't! (Do I?)" episodes.

@SethTisue SethTisue added release-notes worth highlighting in next release notes and removed release-notes worth highlighting in next release notes labels Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants