add default keyword constructor for immutables #6122
Conversation
|
Well, that was easy. :) |
|
No documentation? |
|
This functionality is really, really nice. Works for me like a charm. I'm sure it's clear that there is also a default "copy" constructor as a side-effect, which seems harmless (copying an immutable is a noop, so it's pretty pointless). Any reason this shouldn't be extended to mutable types? |
|
I tried to extend this to mutable types, but it segfaults building sysimg. |
| (let* ((arg-names (safe-field-names field-names field-types)) | ||
| (ctors (list `(function (call ,name ,@arg-names) | ||
| (block (call new ,@arg-names)))))) | ||
| (if (and (or (not mutabl) (eq? mutabl 'false)) |
There was a problem hiding this comment.
This is a bit odd: mutabl is either #f or 'false depending on if the type is defined in the repl or in a script.
There was a problem hiding this comment.
Should be fixed in 86e03d8
|
Some packages explicitly define constructors like this. Will that fail once this lands? |
|
I really like this, and would love to have it for mutables, too. But undefined references might make the implementation for them a major pain. |
|
It's certainly not impossible to handle that – an undef field would remain undef in the new object unless assigned via keyword. It's not super easy to implement in the parser, however. |
|
Hmm. I thought we'd merged this. Any reason not to? I guess we should wait for @JeffBezanson's ok. |
|
Bump. |
|
Also looking forward to this. Just wondering though, will the keyword arg parsing destroy performance when using immutables? |
|
Are we going to do this or what? I periodically think we've already changed this and I want to use the feature, but then I remember that we haven't. |
|
I'd like to, but I think if we do then we should really implement the compile-time keyword sorting optimization. |
|
Ok, good to get a definitive reason for this being on hold. Sounds like a 0.4 thing then. |
|
Would also be good to try again for mutable types and see if the segfault is fixed, or can be fixed. |
nolta
force-pushed
the
mn/5333
branch
2 times, most recently
from
af56ba7
to
42b476a
Compare
jiahao
force-pushed
the
master
branch
3 times, most recently
from
6c7c7e3
to
1a4c02f
Compare
|
Bump. Needs a rebase. Does extending this to mutable still segfault? |
|
Rebased. Mutables still segfault. |
|
This feature would be really nice! And it might be even nicer if default values are supported for the fields of at least immutable types. |
|
Allowing default values, at least with the obvious syntax, would be a breaking change: julia> immutable A
a=5
b::Int
end
julia> names(A)
1-element Array{Symbol,1}:
:b
julia> immutable B
a=5
b::Int
B() = new(a)
end
julia> B()
B(5)Although, presumably this feature is not used a lot. So, I'm all for defaults as well. |
|
Multiple times I have thought default values would work, gotten briefly confused, then frustrated that they don't. Constructors for big types with lots of fields are pretty messy to keep track of at the moment. |
|
We haven't seen much of @nolta lately. Someone who wants this should try adopting the PR, rebasing and submitting anew. |
For example, the type
immutable X
a::Int
b::Int
end
now comes with the extra constructor:
X(x::X; a=x.a, b=x.b) = new(a,b)
Closes #5333.
|
Rebased. |
|
I'd really like to use this feature, but it does not seem to be in the current |
|
Until this gets merged have a look at https://github.com/mauro3/Parameters.jl. It should do all of this and more (except high performance, say you try and use it in a hot inner loop). Please file an issue with Parameters.jl if something is missing. |
|
@mauro3 Thanks, that looks interesting. My use case is in a hot inner loop though. |
|
Keyword functions are currently not very performant (edit: see #9551). I suspect that this PR suffers from that as well. So you probably have to use the non-keyword constructors for a little longer. Also, depending you your use-case this may help: https://github.com/StephenVavasis/Modifyfield.jl |
StefanKarpinski
added
the
speculative
|
@JeffBezanson's objection to this is that this generates a huge number of methods, which is a lot of overhead for a feature that is only wanted sometimes. My objection was why this only applies to immutables rather than mutables as well. A number of ad hoc implementations of this exist in the package ecosystem, however, so some standard solution seems warranted. |
|
Also, would be obviated by the feature of putting an immutable in a |
|
Perhaps we promote one of the package implementations of this to an installed-by-default stdlib? This functionality could cut down a lot of boilerplate in large types for some of the library bindings we have in Base, but most of those should be moved to stdlib modules anyway so adding other stdlib dependencies to them would be okay. |
|
One of them: https://github.com/mauro3/Parameters.jl. |
|
Is this really necessarily 2.0 or can this be added in a non-breaking way in a 1.x release? |
|
Do we want this in base, this seems very much what https://github.com/JuliaObjects/Accessors.jl seems to do. |
|
Base now has |
Implements #5333.