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 isimmutabletype #18168

Closed
wants to merge 4 commits into from
Closed

Conversation

stevengj
Copy link
Member

As discussed on julia-users recently, there is an isimmutable function to check whether a value is immutable, but nothing to check whether a type is immutable, which forces you to look in the internal mutable field of a type. This PR adds an isimmutabletype function to fill this gap.

It also inlines some documentation, and removes an obsolete special-case for Tuple from the isimmutable function.

@eschnett
Copy link
Contributor

Many functions in Julia accept both an object or a type. I would reuse the name isimmutable for both. Or do you think there is much opportunity confusion when people pass in a type, and rather want to have the type of the type examined? Methinks that is an irrelevantly rare case.

@toivoh
Copy link
Contributor

toivoh commented Aug 21, 2016 via email

@TotalVerb
Copy link
Contributor

The corresponding field is called mutable, so why not ismutabletype? One could imagine deprecating isimmutable in favour of ismutable too, for consistency.

@eschnett
Copy link
Contributor

I don't think there are any immutable type objects. I'd be surprised if there is code that checks whether type objects are immutable.

@TotalVerb
Copy link
Contributor

I doubt that there is code that does that specifically for type objects.

But there could be code that is generic across Any that will need to be changed to special-case type objects.

@stevengj stevengj closed this Aug 21, 2016
@stevengj stevengj reopened this Aug 21, 2016
@eschnett
Copy link
Contributor

We can define isimmutable(typeof(x)) to always mean what isimmutabletype(typeof(x)) would mean.

@yuyichao
Copy link
Contributor

I doubt that there is code that does that specifically for type objects.

Which actually means that those code won't really work for types anyway. I'm not sure why does code check mutability but for the two cases I can think of,

  1. DataType is mutable but actually trying to mutate it will be really bad.
  2. !isimmutable may also be used to check if you can use pointer identity, which is also not always true for types.

@kshyatt kshyatt added the domain:types and dispatch Types, subtyping and method dispatch label Aug 22, 2016
@stevengj
Copy link
Member Author

I've updated the PR to make isimmutable(::Type) just another method of isimmutable, rather than a separate function. This seems cleaner, and I agree with @yuyichao that it seems rather unlikely to affect working code.

I can actually only find three packages that use isimmutable(x). StaticArrays uses it in some tests, which won't be affected. @tanmaykm used it in ProtoBuf.jl and Thrift.jl. It looks like it's just using isimmutable for an optimization, so in the unlikely event that you are using those packages to communicate an array of types, hopefully it will still work.

@TotalVerb
Copy link
Contributor

With the introduction of !, I wonder if it is better to provide ismutable to avoid the negative.

@stevengj
Copy link
Member Author

I seriously doubt that isimmutable is critical inner-loop code. It is useful that it can be evaluated at compile time, because it might select between different code branches, but the actual function call itself doesn't seem performance-critical enough to worry about the cost of !.

@TotalVerb
Copy link
Contributor

I mean this from a code readability viewpoint, not an efficiency viewpoint. Perhaps ismutable or !ismutable read better than !isimmutable and isimmutable.

@stevengj
Copy link
Member Author

@StefanKarpinski, I noticed that isimmutable("foo") returns false on 0.6. Was that an intended consequence of the String changes?

@stevengj
Copy link
Member Author

@TotalVerb, from a readability standpoint, immutable is (for now, at least), a keyword in Julia, while mutable is not. It seems clearer to echo the keyword in the function name.

@TotalVerb
Copy link
Contributor

But isimmutable, misleadingly, does not correspond to the immutable keyword, since the predicate includes bitstypes like Int.

@JeffBezanson
Copy link
Sponsor Member

The new String type was declared mutable since it needs to have a stable address.

I recall a discussion somewhere about a function for testing conventional mutability --- for example many types like SparseMatrixCSC are declared with the immutable keyword but contain mutable data and are intended to be mutated. String and DataType are the opposite; declared mutable but not intended to be mutated. Maybe we should introduce such a function.

@TotalVerb
Copy link
Contributor

Maybe such a concept should be separated into separate concepts for each kind of mutability, like for example setindexable for String, AbstractArray, Associative; pushable for AbstractSet.

@JeffBezanson
Copy link
Sponsor Member

I think that's the domain of protocols/traits, and out of scope here.

@@ -156,7 +159,7 @@ This section lists changes that do not have deprecation warnings.
and `step::FloatNN` to rationals and construct a `StepRangeLen`
that internally uses twice-precision arithmetic. These two
outcomes exhibit differences in both precision and speed.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

whitespace

@stevengj
Copy link
Member Author

stevengj commented Feb 3, 2017

Should this be TypeUtils.isimmutable after #20006?

@nalimilan
Copy link
Member

Also, after #20418, it would be more logical to call this ismutable (reversal of the logic discussed above).

Keno added a commit that referenced this pull request Dec 30, 2020
Determines whether a type was declared using `mutable struct`.
Naming follows from the `ismutable` query we already have, analogous
to `isbits`/`isbitstype`. Replaces #18168.
@Keno Keno mentioned this pull request Dec 30, 2020
@Keno
Copy link
Member

Keno commented Dec 30, 2020

I've done a moral rebase of this in #39037.

@Keno Keno closed this Dec 30, 2020
Keno added a commit that referenced this pull request Dec 31, 2020
Determines whether a type was declared using `mutable struct`.
Naming follows from the `ismutable` query we already have, analogous
to `isbits`/`isbitstype`. Replaces #18168.
Keno added a commit that referenced this pull request Dec 31, 2020
Determines whether a type was declared using `mutable struct`.
Naming follows from the `ismutable` query we already have, analogous
to `isbits`/`isbitstype`. Replaces #18168.
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
Determines whether a type was declared using `mutable struct`.
Naming follows from the `ismutable` query we already have, analogous
to `isbits`/`isbitstype`. Replaces JuliaLang#18168.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants