-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
RFC: Update calling-c-and-fortran docs #10799
Conversation
e1c2261
to
22cce85
Compare
to sort using the ``qsort`` function (rather than Julia's built-in ``sort`` | ||
function). Before we worry about calling ``qsort`` and passing arguments, | ||
we need to write a comparison function that works for some arbitrary type | ||
``T``:: | ||
|
||
function mycompare{T}(a::T, b::T) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The T
can be dropped here; we can just write mycompare(a, b)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Updated this example and the corresponding docs.
(Still making my way through the rest of the docs.) |
…on/Ref types, and improve error messages in code
Bump. This needs rebasing apparently. |
3883aa4
to
105fe46
Compare
105fe46
to
c559f49
Compare
Rebased. I also removed references to #2818, which was apparently moved to #10579 and merged. @Keno, @vtjnash, or @JeffBezanson, can you please look over those changes to make sure they're correct? The docs in this area could probably use some more work, but this should be an improvement. |
c559f49
to
f1a7b14
Compare
@@ -86,7 +83,7 @@ of an environment variable, one makes a call like this:: | |||
julia> bytestring(path) | |||
"/bin/bash" | |||
|
|||
Note that the argument type tuple must be written as ``(Ptr{UInt8},)``, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, in 0.4 it should be written as (Cstring,)
since it is a pointer to NUL-terminated bytestring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I've updated the example above and following section, and will push in a second.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type should still be Ptr{UInt8}
. I don't think Cstring
works as expected as a return value? Although it would be nice to make this work...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take it back, since the bytestring
method accepts a Cstring
, it's not crazy to use a Cstring
as a return value. e.g. bytestring(ccall(:strdup, Cstring, (Cstring,), "hello"))
works (although it has a memory leak).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also #12889.
* Formatting changes/fixes * Small wording updates * Removed references to #2818
f1a7b14
to
b7cd8ea
Compare
Thanks, @stevengj. I've fixed the parts you pointed out. |
call signatures accurately reflect those in the C header file. (The `Clang | ||
package` <https://github.com/ihnorton/Clang.jl> can be used to auto-generate | ||
Julia code from a C header file.) | ||
functional; you are responsible for making sure that your Julia types and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functional
looks wrong here too? autocorrect's fault?
Seems like there are some changes here we'd still want, if @kmsquire or anybody else is up for one more rebase/round of updates? |
Redone in #31700 |
This is mostly formatting changes (e.g., many of the tables on the current page are only half-visible, at least on Chrome on OSX), but I've included a few wording updates as I've attempted to understand what the docs are referring to. I also changed most parenthetical notes to footnotes.
This is a work in progress, but any comments/corrections/suggestions appreciated.
Cc: @vtjnash