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

add @pure docstring and example #24817

Closed
wants to merge 8 commits into from
Closed

add @pure docstring and example #24817

wants to merge 8 commits into from

Conversation

miguelraz
Copy link
Contributor

I aggregated Chris Rackauckas's and mbauman's discussion form here

https://discourse.julialang.org/t/pure-macro/3871/3

Into the docstring.

base/expr.jl Outdated
Do not use if the function involved:

- Involves globals, pointers
- Recurses
Copy link
Member

Choose a reason for hiding this comment

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

What’s wrong with recursion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It came up during a Discourse post and @mbauman mentioned the restriction.
https://discourse.julialang.org/t/pure-macro/3871

Copy link
Member

Choose a reason for hiding this comment

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

typejoin is pure and does recursion - seems to be doing OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Will ammend.

base/expr.jl Outdated
Annotates a function to ease type inference by strictly specifying the
output is completely determined by the input.

Note: Misuse can lead to regressions.
Copy link
Member

Choose a reason for hiding this comment

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

I don’t know what regression means. This should just say “Usage can easily lead to whole program corruption or crashes and should be avoided.”

Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to say "Usage" rather than "Misuse"? 😂

base/expr.jl Outdated
- Recurses
- Does not return exactly (`===`) the same result for the same input
- Gets its methods extended after it is called

Copy link
Member

Choose a reason for hiding this comment

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

  • Uses dispatch on one of its arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!
Clarifying: one, or any of its arguments?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, any.

base/expr.jl Outdated
julia> struct Discrete2{apply_map,scale_by_time} end

julia> Base.@pure Discrete2(;apply_map=false,scale_by_time=false) = Discrete{apply_map,scale_by_time}()
Discrete
Copy link
Member

Choose a reason for hiding this comment

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

delete me (duplicate code)

base/expr.jl Outdated
- Gets its methods extended after it is called
- Uses dispatch on one of its arguments

### Example
Copy link
Member

Choose a reason for hiding this comment

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

This example isn't wrong, but it will almost certainly be made obsolete by #24362

base/expr.jl Outdated
- Recurses
- Does not return exactly (`===`) the same result for the same input
- Gets its methods extended after it is called

Copy link
Member

Choose a reason for hiding this comment

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

Yes, any.

base/expr.jl Outdated
@pure

Usage can easily lead to whole program corruption or crashes and should be avoided
by beginners.
Copy link
Member

Choose a reason for hiding this comment

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

"by all users."

@ararslan ararslan added the docs This change adds or pertains to documentation label Nov 29, 2017
@mschauer
Copy link
Contributor

The definition of what @pure actually does got lost during a later commit. (Even if use is discouraged, the doc-string should inform about use and purpose.)

@vtjnash
Copy link
Member

vtjnash commented Nov 29, 2017

Usage is “do not use”: what’s not to like 😜?

@yurivish
Copy link
Contributor

I think the name of the macro is what makes it so tempting to potential users, even if documented as “really, don’t use this.”. Pure is such a nice word, and people think they know what it means. It’s would be a lot of churn, but is renaming the function to something less exciting something within the realm of possibility for 1.0?

@vtjnash
Copy link
Member

vtjnash commented Nov 29, 2017

within the realm of possibility for 1.0?

Sure. I reserve the right to change the name whenever anyone feels so inclined.

@StefanKarpinski
Copy link
Member

I've proposed @hyperpure previously.

@miguelraz
Copy link
Contributor Author

I can put in the work to get @hyperpure pre 1.0 (this means I can work on this during feature freeze, right?), but would very much like at least a canonical example of where @hyperpure is actually useful.
Any help/ pointers?

@StefanKarpinski
Copy link
Member

Just so you're aware, @pure isn't exported so this can be changed at any time.

@miguelraz
Copy link
Contributor Author

That would explain why I couldn't find examples of it in Base -.-

@YingboMa
Copy link
Contributor

YingboMa commented Dec 1, 2017

-> % rg '@pure'
NEWS.md
374:    accessible via the `@pure` constructor `Val(c)`. Functions are defined as

test/inference.jl
400:Base.@pure function fpure(a=rand(); b=rand())
416:# Make sure @pure works for functions using the new syntax
417:Base.@pure (fpure2(x::T) where T) = T
709:Base.@pure b20704(@nospecialize(x)) = f20704(x)
720:Base.@pure g20704(::Int) = 1
725:Base.@pure c20704() = (f20704(1.0); 1)
729:Base.@pure function a20704(x)
980:Base.@pure plus1(x) = x + 1

base/inference.jl
78:    actual::Bool  # if true, we obtained `val` by actually calling a @pure function
1599:@pure function type_typeof(@nospecialize(v))
3606:        # TODO: Improve this analysis; if a function is marked @pure we should really

base/broadcast.jl
153:Base.@pure function BroadcastStyle(a::A, b::B) where {A<:AbstractArrayStyle{M},B<:AbstractArrayStyle{N}} where {M,N}

base/irrationals.jl
31:@pure function convert(::Type{Rational{T}}, x::AbstractIrrational) where T<:Integer
47:@pure function (t::Type{T})(x::AbstractIrrational, r::RoundingMode) where T<:Union{Float32,Float64}
80:@pure function rationalize(::Type{T}, x::AbstractIrrational; tol::Real=0) where T
83:@pure function lessrational(rx::Rational{<:Integer}, x::AbstractIrrational)
84:    # an @pure version of `<` for determining if the rationalization of

base/namedtuple.jl
135:@pure function sym_in(x::Symbol, itr::Tuple{Vararg{Symbol}})
142:@pure function merge_names(an::Tuple{Vararg{Symbol}}, bn::Tuple{Vararg{Symbol}})
152:@pure function merge_types(names::Tuple{Vararg{Symbol}}, a::Type{<:NamedTuple}, b::Type{<:NamedTuple})

base/linalg/linalg.jl
20:    @propagate_inbounds, @pure, reduce, typed_vcat

base/linalg/rowvector.jl
67:@pure check_types(::Type{T1}, ::Type{T2}) where {T1,T2} = T1 === transpose_type(T2) ? nothing :

doc/src/manual/types.md
1347:julia> Base.@pure Val(x) = Val{x}()

There are several places in Julia where @pure is used, so you need to change them too.

@fredrikekre
Copy link
Member

fredrikekre commented Dec 1, 2017

Is it really necessary to change the name? Why would that stop people from using it (incorrectly)?
Edit: Presumably, now that we discourage the use of it in the docstring, people will avoid it.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Apr 13, 2018

Is it really necessary to change the name? Why would that stop people from using it (incorrectly)?

The problem is that a "pure" function has a more familiar sense which this is stricter than. People refer to functions that only depend on their arguments and not on any other mutable program state as being pure. Now technically, if you include the method tables of functions as mutable program state (which it is), then the @pure macro does have this meaning. However, people aren't accustomed to considering method tables of all the functions they use as part of mutable global state, so they don't think of this when applying the @pure macro. Moreover, it's almost impossible for the behavior of a non-trivial method definition not to depend in some way on some method tables, which is why the answer to "is this really pure?" is almost always no, even in seemingly ridiculously simple cases. So the idea behind calling it @hyperpure is to serve as a warning to people that it probably doesn't mean what they think it means. Then again, it's technically accurate and it's not an exported macro, so maybe just documenting the method table part is sufficient.

@mbauman
Copy link
Member

mbauman commented Apr 13, 2018

I would still strongly support this change.

$ ack --type=julia '@pure' ALL_PACKAGES | wc -l
396

Just in my skimming through, I'd imagine that's at least 300 too many.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants