Skip to content

print deprecation warning for bitshift Int by a Long #2968

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
smarter opened this issue Aug 9, 2017 · 10 comments
Closed

print deprecation warning for bitshift Int by a Long #2968

smarter opened this issue Aug 9, 2017 · 10 comments

Comments

@smarter
Copy link
Member

smarter commented Aug 9, 2017

The current situation is pretty bad:

2.11/Dotty:

scala> 1 << 33L
res0: Long = 8589934592

scala> val x = 1
x: Int = 1

scala> x << 33L
res1: Int = 0

2.12 (since scala/scala#4238):

scala> 1 << 33L
res0: Int = 2

scala> val x = 1
x: Int = 1

scala> x << 33L
res1: Int = 2

See also the discussion at https://gitter.im/scala/contributors?at=598b4b3eee5c9a4c5fa61568 where @Ichoran proposed getting rid of Int#<<(x: Long) since it can be so easily misused.

@Ichoran
Copy link

Ichoran commented Aug 9, 2017

To clarify, I find it highly implausible that someone would go to the trouble of making the RHS of a shift be Long (as a constant) unless they intended a Long result. The right-hand-isn't-a-constant case is less clear. But if we're not going to widen everything to Long as was done in 2.11, then the mixed Int/Long case just shouldn't compile because we can't tell whether the RHS was meant to be Int or the LHS was meant to be Long. Better for the user to have to specify which it is.

@som-snytt
Copy link
Contributor

Just wanted to point out that shifting by constant doesn't have to be explicit or obvious. It could be an inherited constant value, etc, so the expression doesn't necessarily indicate an intention to widen.

scala> 1 << 33L
res0: Int = 2

scala> trait T { final val shift = 33L }
defined trait T

scala> 1 << null.asInstanceOf[T].shift
res1: Int = 2

Also, if you name an operation shift, then you must expect it to be shifty.

@Ichoran
Copy link

Ichoran commented Aug 9, 2017

One should write one's contracts especially carefully when dealing with those who are shifty: spell things out.

@DarkDimius
Copy link
Contributor

DarkDimius commented Aug 9, 2017

My opinion on the matter:

  • permit under -language:Scala2, but issue deprecation warning that points to this issue. Compile as 2.12 does.
  • prohibit in Scala3.

@sjrd
Copy link
Member

sjrd commented Aug 9, 2017

The differing behavior in the presence of a constant was a plain bug of the constant folder, fixed in the PR you mentioned. There was, independently, the problem of widening, which was inconsistent with Java and which I fixed in scala/scala#5117.

I think it would be reasonable to deprecate and eventually remove the Int shift Long overload, even in 2.x.

@mdedetrich
Copy link

I agree with @DarkDimius , should be deprecated in Scala2 but removed in Scala3. Implicit number conversions should never happen when doing bitshifts, because they are pretty much never what you want

@som-snytt
Copy link
Contributor

Harmonized at #6366
except difference in warning when folded:

Welcome to Scala 2.13.2 (OpenJDK 64-Bit Server VM, Java 14).
Type in expressions for evaluation. Or try :help.

scala> 0x01030507 << 36L == 271601776
                  ^
       warning: method << in class Int is deprecated (since 2.12.7): shifting a value by a `Long` argument is deprecated (except when the value is a `Long`).
       Call `toInt` on the argument to maintain the current behavior and avoid the deprecation warning.
val res0: Boolean = true

vs

Dotty compiler version 0.25.0-RC2 -- Copyright 2002-2020, LAMP/EPFL
scala> 0x01030507 << 36L == 271601776                                                                                  
val res0: Boolean = true

@bishabosha
Copy link
Member

so it seems to be that the issue here is not printing the deprecation warning for <<(that: Long)

@bishabosha bishabosha changed the title Harmonize behavior of bitshifting with Scala 2.12 print deprecation warning for bitshift Int by a Long Mar 29, 2021
@ckipp01
Copy link
Member

ckipp01 commented May 11, 2023

so it seems to be that the issue here is not printing the deprecation warning for <<(that: Long)

I just tried this out and it does seem to print out the deprecation warning.

Screenshot 2023-05-11 at 09 47 02
//> using scala 3.nightly
//> using option -deprecation

object example:
  val a = 1 << 33L

Am I correct in understanding then that this can be closed?

Also confirmed that the other one @som-snytt pointed out also warns:

Screenshot 2023-05-11 at 09 50 14

@sjrd
Copy link
Member

sjrd commented May 11, 2023

Yes, looks like it can be closed.

@ckipp01 ckipp01 closed this as completed May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants