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

rewrite the C/Fortran calling manual to use @ccall as the "first class" way of calling #45206

Merged
merged 5 commits into from
May 11, 2022

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented May 6, 2022

The macro is much more convenient so I think it makes sense to emphasize that.

Fixes #44038. fixes #34862

@KristofferC KristofferC added the docs This change adds or pertains to documentation label May 6, 2022
@KristofferC KristofferC force-pushed the kc/ccall_to_@ccall branch from b5bcf61 to 57db618 Compare May 6, 2022 09:39
end
```

Here, the input `p` is declared to be of type `Ref{gsl_permutation}`, meaning that the memory
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this based on the discussion in #42480 (comment) (cc @Gnimuc) and that it isn't ideal to have a wrong example like this and then only later describe that it is wrong.

@KristofferC KristofferC force-pushed the kc/ccall_to_@ccall branch from 57db618 to 50cf8cf Compare May 6, 2022 09:41
```

Here is a slightly more complex example that discovers the local machine's hostname.
In this example, the networking library code is assumed to be in a shared library named "libc".
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this because it no longer works to specify "libc":

julia> gethostname()
ERROR: could not load library "libc"
/lib/x86_64-linux-gnu/libc.so: invalid ELF header

https://discourse.julialang.org/t/invalid-elf-header-0-7-0-beta-built-from-source/12054, #450

Copy link
Member

Choose a reason for hiding this comment

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

As the comment here said, this was usually not correct ever, but we want to show the syntax for it here

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, imo, let's show the syntax in a way that is correct then. Like for calling a locally compiled library.

| `@ccall printf("%s = %d\n"::Cstring ; "foo"::Cstring, foo::Cint)::Cint` | `<unavailable>` |
| `@ccall printf("%s = %d\n"::Cstring ; "2 + 2"::Cstring, "5"::Cstring)::Cint` | `ccall(:printf, Cint, (Cstring, Cstring...), "%s = %s\n", "2 + 2", "5")` |
| `<unavailable>` | `ccall(:gethostname, stdcall, Int32, (Ptr{UInt8}, UInt32), hn, length(hn))` |

Copy link
Member

Choose a reason for hiding this comment

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

I think there is also:

Suggested change
| `Base.@assume_effects :total @ccall sin(1::Cdouble,)::Cdouble` | `<unavailable>` |

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think the effect system has been really documented at this point at right now it feels pretty internal so I would probably leave this out until it gets a more official status.

KristofferC and others added 3 commits May 10, 2022 08:50
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
2 participants