Use Val(x) and f(::Val{x}) #22475

andyferris · 2017-06-22T07:26:30Z

@JeffBezanson as discussed. Instead of calling f(Val{x}) and method definitions f(::Type{Val{x}}) where x = ..., we use f(Val(x)) and f(::Val{x}) where x = .... There is a new @pure function for Val(x) -> Val{x}(), a show override, and a docstring/doctest.

~~This is WIP and so far only "upgrades" two functions (ntuple and fill_to_length). I am currently intending to replace~~ I have replaced all uses of Val{x} in Base with the new constructor (except for specific inference tests targeting type parameters).

Also, what does this mean when building? I'm kind-of lost with the documentation system.

WARNING: replacing docs for 'Base.Val :: Union{}' in module 'Base'.

fredrikekre · 2017-06-22T07:40:54Z

Also, what does this mean when building? I'm kind-of lost with the documentation system.
WARNING: replacing docs for 'Base.Val :: Union{}' in module 'Base'.

You are replacing the current docstring here with your new one. You can probably merge them into the same docstring and delete the one in helpdb/Base.jl

martinholters · 2017-06-22T07:57:17Z

Are there any tangible benefits, e.g. in terms of inferability or time needed for inference? (Just curious, not that I'm against this PR if there aren't.)

andyferris · 2017-06-22T14:16:16Z

While I feel it's primarily aesthetic, you can also infer multiple return Val instances from a function. I.e. they can go into a tuple without being inferred as Tuple{Vararg{DataType, N}}.

fredrikekre · 2017-06-24T05:28:24Z

base/deprecated.jl

+@deprecate literal_pow{N}(a,b , ::Type{Val{N}}) literal_pow(f, Val(N))
+#@deprecate IteratorsMD.split(t, V::Type{Val{n}}) IteratorsMD.split(t, Val(n))
+@deprecate sqrtm{T,realmatrix}(A::UpperTriangular{T},::Type{Val{realmatrix}}) sqrtm(A, Val(realmatrix))
+@deprecate lufact(A, pivot::Union{Type{Val{false}}, Type{Val{true}}}) lufact(A, pivot)


The replacement syntax is the same!? Wont this create an infinite loop?

Yes - yes it would :) Thanks @fredrikekre

andyferris · 2017-06-24T06:26:35Z

See also earlier discussion in #9452

andyferris · 2017-06-24T07:00:48Z

I wasn't expected to change 42 45 files, but I think I'm done now. Fingers crossed for CI.

andyferris · 2017-06-24T07:01:45Z

base/deprecated.jl

+@deprecate ntuple{N}(f, ::Type{Val{N}}) ntuple(f, Val(N))
+@deprecate fill_to_length{N}(t, val, ::Type{Val{N}}) fill_to_length(t, val, Val(N))
+@deprecate literal_pow{N}(a,b , ::Type{Val{N}}) literal_pow(f, Val(N))
+#@deprecate IteratorsMD.split(t, V::Type{Val{n}}) IteratorsMD.split(t, Val(n))


the @deprecate macro was not happy with this. Probably unnecessary.

You can @eval the deprecation into the IteratorsMS module.

IIRC, happily @deprecate now accepts a module as well? :)

Nope

julia> @deprecate Base.foo Base.bar ERROR: invalid usage of @deprecate

Could be fixed though.

@eval accepts a module, and you can put the @deprecate in an @eval block

Ah yes, it's @eval module @dep... that I was remembering :).

Thats what I suggested 😄

andyferris · 2017-06-24T07:02:45Z

base/broadcast.jl

@@ -134,7 +134,7 @@ Base.@propagate_inbounds _broadcast_getindex(::Any, A, I) = A[I]
 ## Broadcasting core
 # nargs encodes the number of As arguments (which matches the number
 # of keeps). The first two type parameters are to ensure specialization.
-@generated function _broadcast!(f, B::AbstractArray, keeps::K, Idefaults::ID, A::AT, Bs::BT, ::Type{Val{N}}, iter) where {K,ID,AT,BT,N}
+@generated function _broadcast!(f, B::AbstractArray, keeps::K, Idefaults::ID, A::AT, Bs::BT, ::Val{N}, iter) where {K,ID,AT,BT,N}


Is this public interface (meaning - does this require a @deprecate)?

Should be fine without :).

unless packages have been extending it...

DataArrays, StaticArrays, and IndexedTables define methods for something called _broadcast! but I don't think they're importing the Base one

fredrikekre · 2017-06-24T08:04:51Z

There is an export true/false option for @deprecate, @deprecate old(args...) new(args...) false IIRC

fredrikekre · 2017-06-24T08:30:04Z

fill_to_length, literal_pow and cat_t should not be exported, so you can append a false after the new definitions in deprecated.jl

fredrikekre · 2017-06-24T08:34:30Z

base/deprecated.jl

@@ -1485,6 +1485,23 @@ end
 using .DSP
 export conv, conv2, deconv, filt, filt!, xcorr

+# PR #22475
+@deprecate ntuple{N}(f, ::Type{Val{N}}) ntuple(f, Val(N))


use new where syntax here and below?

@deprecate ntuple(f, ::Type{Val{N}}) where {N} ntuple(f, Val(N))

fredrikekre · 2017-06-24T08:36:48Z

base/essentials.jl

+"""
+    Val(c)
+
+Constructs a `Val{c}()`, which contains no run-time data. Types like this can be used to


"Construct" instead of "Constructs" to comply with the usual phrasing of docstrings?

fredrikekre · 2017-06-24T08:38:43Z

base/essentials.jl

 end

+Val(x) = (@_pure_meta; Val{x}())
+
+show(io::IO, ::Val{x}) where {x} = print(io, "Val($x)")


Why this? Shouldn't we show the object for what it is, i.e. Val{T}() (as it does by default already)

The cool kids like "print it like you write it" these days.

I personally feel this much more aesthetic. There is a doc string for Val which explains the representation.

julia> dot([1, 2], [3, 4]) dot([1, 2], [3, 4])

? 😄

It does kind of make sense to show it the way it's expected to be written, but we don't generally do this in other cases, e.g.

julia> Ref(0) Base.RefValue{Int64}(0)

And also, Val(x) is just for convenience; Val{x}() is the "real" constructor, right? :)

The RefValue example exposes larger differences, e.g. a concrete type, and we allow RefValue{Any}(0) also. In the case of Val the relation is (more-or-less) one-to-one; I wouldn't have done it otherwise and in my opinion the printing of RefValue is 100% correct.

I think I'm ok with either printing in this case.

andyferris · 2017-06-29T21:57:12Z

There is a single spawn test failure on OSX throwing an InterruptException on travis. I'm guessing this is unrelated.

Other than that - is this good to go?

tkelman · 2017-06-29T22:01:48Z

base/deprecated.jl

+@deprecate literal_pow{N}(a,b , ::Type{Val{N}}) literal_pow(f, Val(N)) false
+@eval IteratorsMD @deprecate split{n}(t, V::Type{Val{n}}) split(t, Val(n)) false
+@deprecate sqrtm{T,realmatrix}(A::UpperTriangular{T},::Type{Val{realmatrix}}) sqrtm(A, Val(realmatrix))
+@deprecate lufact(::AbstractMatrix, ::Type{Val{false}}) lufact(A, Val(false))


missing A on the input ?

tkelman · 2017-06-29T22:02:07Z

base/deprecated.jl

+# PR #22475
+@deprecate ntuple{N}(f, ::Type{Val{N}}) ntuple(f, Val(N))
+@deprecate fill_to_length{N}(t, val, ::Type{Val{N}}) fill_to_length(t, val, Val(N)) false
+@deprecate literal_pow{N}(a,b , ::Type{Val{N}}) literal_pow(f, Val(N)) false


strange spacing on the inputs, f not defined?

tkelman · 2017-06-29T22:02:38Z

base/deprecated.jl

+@deprecate cat{N}(::Type{Val{N}}, A::AbstractArray...) cat(Val(N), A...)
+@deprecate cat{N}(::Type{Val{N}}, A::SparseArrays._SparseConcatGroup...) cat(Val(N), A...)
+@deprecate cat{N}(::Type{Val{N}}, A::SparseArrays._DenseConcatGroup...) cat(Val(N), A...)
+@deprecate cat_t{N,T}(::Type{Val{N}}, ::Type{T}, A, B) cat(Val(N), T, A, B) false


should this call cat_t ?

tkelman · 2017-06-29T22:03:26Z

base/essentials.jl

+Return `Val{c}()`, which contains no run-time data. Types like this can be used to
+pass the information between functions through the value `c`, which must be an `isbits`
+value. The intent of this construct is to be able to dispatch on constants directly (at
+compile time) without having test the value of the constant at run time.


having to test

tkelman · 2017-06-29T22:04:12Z

base/essentials.jl

+compile time) without having test the value of the constant at run time.
+
+### Example
+


use

# Examples ```jldoctest

single #, plural Examples, no blank

tkelman · 2017-06-29T22:10:36Z

base/linalg/cholesky.jl

@@ -208,7 +208,7 @@ chol(x::Number, args...) = ((C, info) = _chol!(x, nothing); @assertposdef C info
 # cholfact!. Destructive methods for computing Cholesky factorization of real symmetric
 # or Hermitian matrix
 ## No pivoting (default)
-function cholfact!(A::RealHermSymComplexHerm, ::Type{Val{false}}=Val{false})
+function cholfact!(A::RealHermSymComplexHerm, ::Val{false}=Val(false))


cholfact! and cholfact are missing from news and deprecations

tkelman · 2017-06-29T22:14:50Z

base/reshapedarray.jl

-function reshape(parent::AbstractArray, ndims::Type{Val{N}}) where N
-    reshape(parent, rdims(Val{N}, indices(parent)))
+reshape(parent::AbstractArray{T,N}, ndims::Val{N}) where {T,N} = parent
+function reshape(parent::AbstractArray, ndims::Val{N}) where N


mention in news

fredrikekre · 2017-06-30T18:26:51Z

Should do something with these too I think

julia/base/deprecated.jl

Lines 1356 to 1362 in 71d7c16

    
           # PR #22188 
        
           @deprecate cholfact!(A::StridedMatrix, uplo::Symbol, ::Type{Val{false}}) cholfact!(Hermitian(A, uplo), Val{false}) 
        
           @deprecate cholfact!(A::StridedMatrix, uplo::Symbol) cholfact!(Hermitian(A, uplo)) 
        
           @deprecate cholfact(A::StridedMatrix, uplo::Symbol, ::Type{Val{false}}) cholfact(Hermitian(A, uplo), Val{false}) 
        
           @deprecate cholfact(A::StridedMatrix, uplo::Symbol) cholfact(Hermitian(A, uplo)) 
        
           @deprecate cholfact!(A::StridedMatrix, uplo::Symbol, ::Type{Val{true}}; tol = 0.0) cholfact!(Hermitian(A, uplo), Val{true}, tol = tol) 
        
           @deprecate cholfact(A::StridedMatrix, uplo::Symbol, ::Type{Val{true}}; tol = 0.0) cholfact(Hermitian(A, uplo), Val{true}, tol = tol)

since they will be in the same release as this, or they will suggest something that is also deprecated. Should be enough to updtae the replacement for them. There are some other Val usage in the 0.6 section of deprecated.jl but should not matter.

JeffBezanson · 2017-07-03T17:50:02Z

👍
There is some discussion of this in the manual that needs to be updated as well. Otherwise looks good to go.

tkelman · 2017-07-03T17:54:02Z

The timeout on 64 bit Linux Travis is a bit unexpected.

tkelman · 2017-07-03T17:54:21Z

@nanosoldier runbenchmarks(ALL, vs = ":master")

nanosoldier · 2017-07-03T20:46:11Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

Replaces `f(Val{x})` on call sites with `f(Val(x))`, using a new `@pure` function `Val(x) = Val{x}()`. This simplifies the method definitions from `f(::Type{Val{x}}) where x` to `f(::Val{x}) where x`. This form also has the advantage that multiple singleton instances can be put in a tuple and inference will work (similarly with multiple-return functions).

andyferris · 2017-07-05T12:04:15Z

Thanks everyone for reviewing!

I have rebased, and updated the manual to reflect this change.

andyferris · 2017-07-05T13:45:13Z

Appveyor error is something about pkg and libgit2 on Windows...

JeffBezanson · 2017-07-06T19:44:39Z

Merge?

andyferris · 2017-07-06T21:37:17Z

Yes, please (I can't do it).

Introduced in JuliaLang/julia#22475

fredrikekre reviewed Jun 24, 2017

View reviewed changes

andyferris mentioned this pull request Jun 24, 2017

Add wrapper for passing constants through type inference #9452

Merged

andyferris force-pushed the master branch from 6e61a0a to 85d730e Compare June 24, 2017 06:59

andyferris changed the title ~~WIP Use Val(x) and f(::Val{x})~~ Use Val(x) and f(::Val{x}) Jun 24, 2017

andyferris commented Jun 24, 2017

View reviewed changes

fredrikekre reviewed Jun 24, 2017

View reviewed changes

andyferris force-pushed the master branch 3 times, most recently from 30463e4 to 51998dc Compare June 29, 2017 14:13

tkelman reviewed Jun 29, 2017

View reviewed changes

andyferris force-pushed the master branch 2 times, most recently from 2bdbb63 to 76c4a2b Compare July 3, 2017 06:32

andyferris force-pushed the master branch from 76c4a2b to f0a91f7 Compare July 5, 2017 11:42

Update manual for Val(x) instance usage

2be13f3

andyferris force-pushed the master branch from 5ad5d93 to 2be13f3 Compare July 5, 2017 12:05

JeffBezanson merged commit 259996c into JuliaLang:master Jul 6, 2017

andreasnoack mentioned this pull request Jul 13, 2017

Add atsign-compat support for CartesianRange JuliaLang/Compat.jl#377

Merged

ararslan added a commit to JuliaLang/Compat.jl that referenced this pull request Jul 19, 2017

Add Val(x)

280c2fc

Introduced in JuliaLang/julia#22475

ararslan mentioned this pull request Jul 19, 2017

Add Val(x) JuliaLang/Compat.jl#381

Merged

ararslan added a commit to JuliaLang/Compat.jl that referenced this pull request Jul 19, 2017

Add Val(x)

766c379

Introduced in JuliaLang/julia#22475

ararslan added a commit to JuliaLang/Compat.jl that referenced this pull request Jul 19, 2017

Add Val(x)

a3bfce5

Introduced in JuliaLang/julia#22475

ajkeller34 mentioned this pull request Jul 28, 2017

Make @fastmath aware of Base.literal_pow #21099

Closed

tkoolen mentioned this pull request Aug 3, 2017

Add more functions implemented with [map]reduce and [map]reducedim JuliaArrays/StaticArrays.jl#263

Merged

yuyichao mentioned this pull request Aug 4, 2017

Drop 0.4 and 0.7 fixes JuliaLang/Compat.jl#385

Merged

		compile time) without having test the value of the constant at run time.

		### Example

Use Val(x) and f(::Val{x}) #22475

Use Val(x) and f(::Val{x}) #22475

Conversation

andyferris commented Jun 22, 2017 • edited Loading

fredrikekre commented Jun 22, 2017 • edited Loading

martinholters commented Jun 22, 2017

andyferris commented Jun 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyferris commented Jun 24, 2017

andyferris commented Jun 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredrikekre commented Jun 24, 2017

fredrikekre commented Jun 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyferris Jun 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyferris commented Jun 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredrikekre commented Jun 30, 2017

JeffBezanson commented Jul 3, 2017

tkelman commented Jul 3, 2017

tkelman commented Jul 3, 2017

nanosoldier commented Jul 3, 2017

andyferris commented Jul 5, 2017

andyferris commented Jul 5, 2017

JeffBezanson commented Jul 6, 2017

andyferris commented Jul 6, 2017

andyferris commented Jun 22, 2017 •

edited

Loading

fredrikekre commented Jun 22, 2017 •

edited

Loading

andyferris commented Jun 24, 2017 •

edited

Loading

fredrikekre commented Jun 24, 2017 •

edited

Loading

andyferris Jun 26, 2017 •

edited

Loading

andyferris commented Jun 29, 2017 •

edited

Loading