WIP: change lowering of ccall cconvert arguments #9913
Conversation
and added tests for two of those cases
| @@ -100,6 +101,7 @@ end | |||
| function study(regex::Ptr{Void}, options::Integer) | |||
| # NOTE: options should always be zero in current PCRE | |||
| errstr = Array(Ptr{UInt8},1) | |||
| errstr[1] = C_NULL | |||
There was a problem hiding this comment.
Looks like a bug fix? Could this go directly to master?
There was a problem hiding this comment.
mostly this was caused because I accidentally was passing a copy of the array in the pointer, rather than the array itself (during work on this PR). it shouldn't strictly be necessary on master (since pcre should set this variable)
|
Really nice change. The amount of code deletion speaks for itself :) |
| *needStackRestore = true; | ||
| Value *jvt = emit_typeof(jv); | ||
| Value *p; | ||
| BasicBlock *mutableBB = BasicBlock::Create(getGlobalContext(),"is-mutable",ctx->f); |
There was a problem hiding this comment.
This is a great hack. It looks correct to me; hope I'm right about that :)
There was a problem hiding this comment.
(added WIP title)
yeah, i'll definately have to do more testing on this. i probably will hold off on merging until I get a few more of the ccall-related PR's ready. the runtime code branch is identical to the compile-time code branch directly above it, however.
There was a problem hiding this comment.
oops, it /was/ slightly broken.
vtjnash
changed the title
indeed. i'm also working on deprecating |
|
Could we remove the extra |
|
it's possibly there to help inference correctly count usages correctly? i'm not really sure |
|
Does the travis failure make any sense to you? If neither of us can figure out why the extra root is there we should try removing it. |
|
yeah, that should be fixed in my followup fix (two bugs: i was looking for |
…s symmetric, the rhs shouldn't be stored with symmetric flag set.
fix bug in worker-to-worker connection setup. closes #9951
parallel test fixup [ci skip]
fix rational vs float comparisons (attempt 2)
remove unwarranted handling of SIGPIPE instead of ignoring it.
Conflicts: src/julia-syntax.scm
Not exported. Fixes #9957.
Fix `show_method_candidates` so it do not fail for methods with zero args.
Inbuilt managers throw on unused arguments. Closes #9958
Ref: f9da849 ReST tables must have not have text in between columns delimited by headers.
more fixes for worker-worker connection setups
Correct rounding mode after calling fma
ccall(:a, B, (C,), d) is now lowered as:
d#1 = cconvert_gcroot(C, d)
Expr(:call, :ccall, :a, B, (C,), cconvert(C, d#1), d#1)
This allows implementation of the special-case ccall lowering logic in Julia
and still create a gcroot for it as needed
the Ref{T} type is the new supertype of Ptr{T}. the ccall emission
turns a Ref{T} into a Ptr{T}, as if the &addressOf operator had been
applied. the specific rules for this will be moved from the current
gist to the documentation at a later point.
vtjnash
force-pushed
the
jn/ccall_cconvert2
branch
from
41a370e
to
52ce45a
Compare
ccall(:a, B, (C,), d)is now lowered as:This allows implementation of the special-case ccall lowering logic in Julia.
It also means that the fall-back cconvert method is now a no-op, with the main
work being done by convert. But it also means we get a GC root for both
the result of cconvert and convert (as needed).
ps: this was the motivation for #9729