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 bin, oct, dec, hex, and base in favor of string and keyword args #25804

Merged
merged 2 commits into from
Mar 2, 2018

Conversation

JeffBezanson
Copy link
Member

Updated version of #25717

@JeffBezanson JeffBezanson added the deprecation This change introduces or involves a deprecation label Jan 29, 2018
@JeffBezanson JeffBezanson added this to the 1.0 milestone Jan 29, 2018
@Keno
Copy link
Member

Keno commented Jan 30, 2018

Looks like strings/basic is failing.

@JeffBezanson JeffBezanson force-pushed the bramtayl-base_keywords branch 3 times, most recently from 9786193 to 4d277b4 Compare January 30, 2018 23:37
@JeffBezanson
Copy link
Member Author

Now stuck on a mysterious inference failure. Haven't seen this one in a long time:

Error During Test at C:\projects\julia\julia-0aa48c105f\share\julia\test\numbers.jl:2870
  Got an exception of type ErrorException outside of a @test
  return type Rational{UInt128} does not match inferred return type Rational{_1} where _1

@StefanKarpinski
Copy link
Member

Bump.

@JeffBezanson
Copy link
Member Author

Still no dice on the mystery inference problem.

@Keno
Copy link
Member

Keno commented Feb 27, 2018

Rebased. I don't see the mystery inference failure locally, so maybe it went away.

@JeffBezanson
Copy link
Member Author

Still happening on travis i686 :(

@martinholters
Copy link
Member

On this branch, built for i686:

julia> Core.Compiler.return_type(+, Tuple{Rational{UInt128}, Rational{UInt128}})
Rational{_1} where _1

julia> Core.Compiler.return_type(Base.divgcd, Tuple{UInt128, UInt128})
Tuple{Any,Any}

julia> Core.Compiler.return_type(div, Tuple{UInt128, UInt128})
Any

julia> code_warntype(div, Tuple{UInt128,UInt128})
Variables:
  x::UInt128
  y::UInt128

Body:
  begin
      Core.SSAValue(0) = $(Expr(:invoke, MethodInstance for BigInt(::UInt128), :(Base.BigInt), :(x)))::BigInt
      Core.SSAValue(1) = $(Expr(:invoke, MethodInstance for BigInt(::UInt128), :(Base.BigInt), :(y)))::BigInt
      # meta: location gmp.jl div 411
      Core.SSAValue(5) = $(Expr(:invoke, MethodInstance for tdiv_q(::BigInt, ::BigInt), Base.GMP.MPZ.tdiv_q, Core.SSAValue(0), Core.SSAValue(1)))::BigInt
      # meta: pop location
      Core.SSAValue(3) = $(Expr(:invoke, MethodInstance for UInt128(::BigInt), :(Base.UInt128), Core.SSAValue(5)))::UInt128
      return Core.SSAValue(3)
  end::UInt128

julia> Core.Compiler.return_type(div, Tuple{UInt128, UInt128})
UInt128

julia> Core.Compiler.return_type(Base.divgcd, Tuple{UInt128, UInt128})
Tuple{Any,Any}

julia> code_warntype(Base.divgcd, Tuple{UInt128,UInt128})
# ...
  end::Tuple{UInt128,UInt128}

julia> Core.Compiler.return_type(Base.divgcd, Tuple{UInt128, UInt128})
Tuple{Any,Any}

WAT?

(My guess for the platform-specificity here is that div(::UInt128, ::UInt128) uses BigInt internally on 32bit platforms.)

It looks like this can be worked around by a type assertion (to be pushed momentarily), but it would be nice to figure out what causes this weird behavior.

@martinholters
Copy link
Member

I can confirm the source of the platform-specificity. With

--- a/base/int.jl
+++ b/base/int.jl
@@ -645,7 +645,7 @@ widemul(x::Number,y::Bool) = x * y
 
 ## wide multiplication, Int128 multiply and divide ##
 
-if Core.sizeof(Int) == 4
+if true
     function widemul(u::Int64, v::Int64)
         local u0::UInt64, v0::UInt64, w0::UInt64
         local u1::Int64, v1::Int64, w1::UInt64, w2::Int64, t::UInt64

the same failures show on 64bit systems (without the type-assertion work-around, of course).

@JeffBezanson JeffBezanson merged commit d480d1b into master Mar 2, 2018
@JeffBezanson JeffBezanson deleted the bramtayl-base_keywords branch March 2, 2018 04:53
mbauman added a commit that referenced this pull request Mar 5, 2018
…luenonscalarindexedassignment

* origin/master: (28 commits)
  fix an optimizer bug in `invoke` with non-constant functions (#26301)
  lower top-level statements in such a way that the front-end knows (#26304)
  Make sure Sockets page has h1 header (#26315)
  fix doctests, and make them less prone to errors (#26275)
  FIx intro to manual chapter on types (#26312)
  Add a missing "that" (#26313)
  fix docstring for code_llvm (#26266)
  Remove the examples/ folder (#26153)
  download cert.pem from deps-getall, fixes #24903 (#25344)
  Slight update to doc string for at-enum to refer to instances (#26208)
  performance tweak in reverse(::String) (#26300)
  remove references to `TCPSocket` in Base docstrings (#26298)
  Deprecate adding integers to CartesianIndex (#26284)
  Deprecate conj(::Any), add real(::Missing) and imag(::Missing) (#26288)
  fix #26267, regression in `names` with `imported=false` (#26293)
  fix #25857, `typeinfo` problem in showing arrays with abstract tuple types (#26289)
  Add adjoint(::Missing) method (#25502)
  Use lowered form in logging macro (#26291)
  deprecate bin, oct, dec, hex, and base in favor of `string` and keyword args (#25804)
  deprecate `spawn(cmd)` to `run(cmd, wait=false)` (#26130)
  ...
Keno pushed a commit that referenced this pull request Jun 5, 2024
…rd args (#25804)

* deprecate bin, oct, dec, hex, and base in favor of `string` and keyword args

* Work around inference problem in `div(::UInt128, ::UInt128)`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants