Conversation
|
Fixed size arrays can now be emulated by tuples. Anyway, I think we should merge this right after we tag 0.3. There is outstanding questions about the interface for cfunction, but since we intend 0.4 to be breaking anyway, we can figure that out later. |
|
*mostly emulated by tuples – someone still should make that true for tuples inside of types. (hint, hint) i meant to add the 0.4 tag, since i was intending to have this ready to merge this shortly into that...done now julep proposal for the interface changes: https://gist.github.com/vtjnash/7296634 |
|
Yeah, we need to do that. |
There was a problem hiding this comment.
a green X?
but yes, that's a good reminder that we need to scan through this and remove any debug code
base/reflection.jl
Outdated
There was a problem hiding this comment.
@Keno which was correct? I suspect this wasn't supposed to change.
There was a problem hiding this comment.
I'm not sure right now. I seem to recall this function also being implemented in C, so we should match whatever is done there.
There was a problem hiding this comment.
Line 542 in 31eb8d4
seems to indicate this change was correct?
There was a problem hiding this comment.
Yeah, that matches my recollection.
There was a problem hiding this comment.
t.names!=() implies !t.abstract, so I believe the two versions are equivalent.
|
Nice!! +1 |
|
I'd like to go ahead and merge this very soon, so we have as much time as possible to encounter and fix any possible issues. |
|
I assumed you would, which is why I wrote in big letters that more work needs to be done over the next few weeks to finish this (WIP tag added also) |
|
Well, let's sit down via IRC this weekend and hammer out the cfunction changes then. That's what's been blocking this for about a year, so we should finally just do it. |
|
i'm busy this weekend. either comment on the gist, propose your own, or wait for next weekend. i don't think the API questions were blocking this, I just didn't want to steal your pull request to make the necessary additions, since I couldn't be sure if you were going to get back to it. |
|
Well, the reason this wasn't merged a year ago is that the cfunction API wasn't done. |
6c7c7e3 to
1a4c02f
Compare
|
Any progress on this? |
|
Lots. It's currently blocked by a number of other PRs that are needed first however. |
b8db56e to
00756be
Compare
|
Awesome job improving the ccall docs, Jameson. |
There was a problem hiding this comment.
Complex shouldn't be here; it's not in this module.
There was a problem hiding this comment.
i wonder if it should move here (or had at some points in the life of this PR), since it is now an influential type on ccall ABI generation
There was a problem hiding this comment.
Not a big deal; the run time system already refers to some stuff in Base.
|
This is epic. Especially love the wonderfully detailed documentation. |
There was a problem hiding this comment.
(I wish that cfunction would just call convert and add the typeassert for you.)
There was a problem hiding this comment.
Can you edit the top post and add it as future work? It wouldn't actually be that hard to add anymore.
|
Typo?
|
Addresses comments: #7906 (comment) #7906 (comment)
this is
a first cut at(done)rebasing #3466 onto master(also done) ready-to-merge patch to improve ccall/cfunction compatibility. @Keno let me know if you see anything that looks wrong.To keep track of issues (from comments) from the previous PR:
Need to figure out what to do with fixed-size arrays.(will be done later)(only cfunction updates remain)future work:
jl_apply_genericcall (possible sketch of implementation : https://gist.github.com/vtjnash/019b4d5d63b64a772a31)cfunction(f, T, ...)to automatically callconvert(T, ...)::Ton the return value off.convert(::Type{T}, x::Ref{T})methodNOTE: please DO NOT merge this until the cfunction API changes are finalized. (i don't want to need to update Gtk.jl twice)