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.

Sign up for GitHub

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

#34518 - rename isimmutable to ismutable #34652

Merged
merged 9 commits into from
Feb 7, 2020
Merged

#34518 - rename isimmutable to ismutable #34652

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8faa424
rename isimmutable to ismutable
ssikdar1 Feb 4, 2020
6b76c59
remove depwarn and move isimmutable to the bottom of deprecated.jl
ssikdar1 Feb 4, 2020
a77d03a
add warning message to isimmutable documentation
ssikdar1 Feb 4, 2020
ca4d104
add NEWS item
ssikdar1 Feb 4, 2020
a252f5e
add Julia version to deprecation warning in docstring
ssikdar1 Feb 5, 2020
eed8b67
change isimmutable to be defined in terms of ismutable
ssikdar1 Feb 5, 2020
28f7f9f
move isimmutable export to deprecated.jl
ssikdar1 Feb 5, 2020
aba863f
change order of ismutable isimmutable in docs
ssikdar1 Feb 5, 2020
dc724e5
Merge branch 'master' into immutable-refactor
ssikdar1 Feb 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion base/compiler/abstractinterpretation.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ function const_prop_profitable(@nospecialize(arg))
isconstType(b) && return true
const_prop_profitable(b) && return true
end
elseif !isa(arg, Const) || (isa(arg.val, Symbol) || isa(arg.val, Type) || (!isa(arg.val, String) && isimmutable(arg.val)))
elseif !isa(arg, Const) || (isa(arg.val, Symbol) || isa(arg.val, Type) || (!isa(arg.val, String) && !ismutable(arg.val)))
# don't consider mutable values or Strings useful constants
return true
end
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/ssair/passes.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ function lift_leaves(compact::IncrementalCompact, @nospecialize(stmt),
elseif isa(leaf, Union{Argument, Expr})
return nothing
end
isimmutable(leaf) || return nothing
!ismutable(leaf) || return nothing
isdefined(leaf, field) || return nothing
val = getfield(leaf, field)
is_inlineable_constant(val) || return nothing
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/tfuncs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ function isdefined_tfunc(@nospecialize(args...))
return Const(true)
elseif isa(arg1, Const)
arg1v = (arg1::Const).val
if isimmutable(arg1v) || isdefined(arg1v, idx) || (isa(arg1v, DataType) && is_dt_const_field(idx))
if !ismutable(arg1v) || isdefined(arg1v, idx) || (isa(arg1v, DataType) && is_dt_const_field(idx))
return Const(isdefined(arg1v, idx))
end
end
Expand Down Expand Up @@ -722,7 +722,7 @@ function getfield_tfunc(@nospecialize(s00), @nospecialize(name))
if !(isa(nv,Symbol) || isa(nv,Int))
return Bottom
end
if (isa(sv, SimpleVector) || isimmutable(sv)) && isdefined(sv, nv)
if (isa(sv, SimpleVector) || !ismutable(sv)) && isdefined(sv, nv)
return AbstractEvalConstant(getfield(sv, nv))
end
end
Expand Down
1 change: 1 addition & 0 deletions base/exports.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,7 @@ export
isbits,
isequal,
isimmutable,
Copy link
Member

Choose a reason for hiding this comment

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

You could move this export to where the function is defined in deprecated.jl such that we don't forget to remove it once the function definition is removed (that has happened before).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

export moved to deprecated.jl in 28f7f9f

ismutable,
isless,
ifelse,
objectid,
Expand Down
4 changes: 2 additions & 2 deletions base/gcutils.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ end
```
"""
function finalizer(@nospecialize(f), @nospecialize(o))
if isimmutable(o)
if !ismutable(o)
error("objects of type ", typeof(o), " cannot be finalized")
end
ccall(:jl_gc_add_finalizer_th, Cvoid, (Ptr{Cvoid}, Any, Any),
Expand All @@ -37,7 +37,7 @@ end

function finalizer(f::Ptr{Cvoid}, o::T) where T
@_inline_meta
if isimmutable(o)
if !ismutable(o)
error("objects of type ", typeof(o), " cannot be finalized")
end
ccall(:jl_gc_add_ptr_finalizer, Cvoid, (Ptr{Cvoid}, Any, Ptr{Cvoid}),
Expand Down
23 changes: 22 additions & 1 deletion base/reflection.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,28 @@ julia> isimmutable([1,2])
false
```
"""
isimmutable(@nospecialize(x)) = (@_pure_meta; !typeof(x).mutable)
function isimmutable(@nospecialize(x))
Copy link
Member

Choose a reason for hiding this comment

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

We generally don't start introducing "live" depwarns of exported functions in the middle of a stable release. Instead you should move this to the end of base/deprecated.jl but don't actually issue the depwarn. Most or all of the "live" depwarns there are for types or methods that were never exported in the first place and therefore not protected by the "no breaking changes" rule. Thus, those deprecations are not necessary but are there as a kindness. In contrast, this is part of Julia's public API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok! moved isimmutable from reflections.jl to the bottom of deprecated.jl in 6b76c59

depwarn(" isimmutable is deprecated use ismutable instead ", :isimmutable)
return (@_pure_meta; !typeof(x).mutable)
end

"""
ismutable(v) -> Bool

Return `true` iff value `v` is mutable. See [Mutable Composite Types](@ref)
for a discussion of immutability. Note that this function works on values, so if you give it
a type, it will tell you that a value of `DataType` is mutable.

# Examples
```jldoctest
julia> ismutable(1)
false

julia> ismutable([1,2])
true
```
"""
ismutable(@nospecialize(x)) = (@_pure_meta; typeof(x).mutable)

"""
isstructtype(T) -> Bool
Expand Down
1 change: 1 addition & 0 deletions doc/src/base/base.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ Base.isdispatchtuple

```@docs
Base.isimmutable
Base.ismutable
Copy link
Member

Choose a reason for hiding this comment

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

Maybe switch the order and put ismutable first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

order switched in aba863f

Base.isabstracttype
Base.isprimitivetype
Base.issingletontype
Expand Down
4 changes: 2 additions & 2 deletions test/reflection.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ not_const = 1
@test isconst(@__MODULE__, :not_const) == false
@test isconst(@__MODULE__, :is_not_defined) == false

@test isimmutable(1) == true
@test isimmutable([]) == false
@test ismutable(1) == false
@test ismutable([]) == true

## find bindings tests
@test ccall(:jl_get_module_of_binding, Any, (Any, Any), Base, :sin)==Base
Expand Down