Skip to content

deprecate bin, oct, dec, hex, and base in favor of string and keyword args#25804

Merged
JeffBezanson merged 2 commits intomasterfrom
bramtayl-base_keywords
Mar 2, 2018
Merged

deprecate bin, oct, dec, hex, and base in favor of string and keyword args#25804
JeffBezanson merged 2 commits intomasterfrom
bramtayl-base_keywords

Conversation

@JeffBezanson
Copy link
Copy Markdown
Member

@JeffBezanson JeffBezanson commented Jan 29, 2018

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
@JeffBezanson JeffBezanson mentioned this pull request Jan 29, 2018
Closed
@JeffBezanson JeffBezanson force-pushed the bramtayl-base_keywords branch from 6731389 to 4b51121 Compare January 29, 2018 22:43
@Keno
Copy link
Copy Markdown
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
Copy Markdown
Member Author

JeffBezanson commented Jan 31, 2018

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

@JeffBezanson JeffBezanson force-pushed the bramtayl-base_keywords branch 4 times, most recently from ed391ad to bf2cb09 Compare February 5, 2018 21:09
@andyferris andyferris mentioned this pull request Feb 5, 2018
Open
@JeffBezanson JeffBezanson force-pushed the bramtayl-base_keywords branch from bf2cb09 to b7ca5d8 Compare February 6, 2018 22:09
@StefanKarpinski
Copy link
Copy Markdown
Member

StefanKarpinski commented Feb 14, 2018

Bump.

@JeffBezanson JeffBezanson force-pushed the bramtayl-base_keywords branch from b7ca5d8 to 10f5d12 Compare February 14, 2018 17:52
@JeffBezanson
Copy link
Copy Markdown
Member Author

JeffBezanson commented Feb 14, 2018

Still no dice on the mystery inference problem.

@Keno Keno force-pushed the bramtayl-base_keywords branch from 10f5d12 to 9db1e6b Compare February 27, 2018 20:34
@Keno
Copy link
Copy Markdown
Member

Keno commented Feb 27, 2018

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

@JeffBezanson
Copy link
Copy Markdown
Member Author

JeffBezanson commented Feb 28, 2018

Still happening on travis i686 :(

@martinholters
Copy link
Copy Markdown
Member

martinholters commented Feb 28, 2018

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
Copy Markdown
Member

martinholters commented Feb 28, 2018

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 force-pushed the bramtayl-base_keywords branch from 4d0ede7 to c002de7 Compare March 1, 2018 22:32
@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
@mbauman
Merge remote-tracking branch 'origin/master' into mb/deprecatemultiva…
95a78df 
…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)
  ...
@mbauman mbauman mentioned this pull request May 15, 2018
Closed
@martinholters martinholters mentioned this pull request Nov 15, 2018
Closed
Keno pushed a commit that referenced this pull request Jun 5, 2024
@JeffBezanson
deprecate bin, oct, dec, hex, and base in favor of string and keywo…
bcf3132 
…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

Reviewers

No reviews

Assignees

No one assigned

Labels

deprecation This change introduces or involves a deprecation

Projects

None yet

Milestone

0.7

Development

Successfully merging this pull request may close these issues.

5 participants

@JeffBezanson @Keno @StefanKarpinski @martinholters @bramtayl