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

remove use of Ref{Void} as a struct field type #43

Closed
JeffBezanson opened this issue Feb 2, 2016 · 6 comments
Closed

remove use of Ref{Void} as a struct field type #43

JeffBezanson opened this issue Feb 2, 2016 · 6 comments

Comments

@JeffBezanson
Copy link
Collaborator

I believe we need more changes of the kind made in #40. For example _35_3_Providing_the_function_to_solve.jl still has a use of Ref{Void} in a struct. This is only correct if you want the struct to contain exactly a pointer to the object nothing. @vtjnash am I right about that?

Problems arise when storing a Ptr{Void} to such a field, because Ptr{Void} is no-op convertible to Ref{Void}, so the field ends up pointing to a boxed pointer. This strikes me as a bug in the type safety of Ref.

@vtjnash
Copy link
Collaborator

vtjnash commented Feb 2, 2016

This strikes me as a bug in the type safety of Ref.

true, this may be worth a special case for Ref. In C, the void type is more nearly Union{} in Julia. The common mapping in Julia to Void has some slightly different behaviors (such as this one).

@JeffBezanson
Copy link
Collaborator Author

I don't fully agree with that; Union{} is closer to a noreturn declaration. void in C is pretty close to being a normal observable value.

This problem would occur with any type; assigning a Ptr{Int} to a Ref{Int} field would do the same thing. The only fix I can think of so far is to throw an error for converting a Ptr to a Ref. Ptr could still be a subtype of Ref. That seems to handle the contexts where Ptr can vs. can't be substituted as a Ref.

@vtjnash
Copy link
Collaborator

vtjnash commented Feb 3, 2016

If you declare the field type wrong, the code will be wrong. There's nothing unique to Ref there, since the same applies to any other error in field declaration. otoh, if you declare the fields correctly, then you can't substitute Ref for Ptr (since that requires an unsafe_convert), which is the intended direction for usage-safety.

void is not constructible, cannot be used as a field, and is convertible to/from all other types but cannot itself be constructed: http://programmers.stackexchange.com/a/254191
In C++, void &x would be an error because void cannot be constructed, which is more similar to how Union{} behaves in Julia. In particular, the C standard specifies that the presence of the void type is special in that it does not have a value (usage of it would be an error), but the code must still be executed for side-effects.

@JeffBezanson
Copy link
Collaborator Author

I think this problem reflects some unclarity about when to use Ref instead of Ptr. The docs basically say "use Ref". Maybe all we need to do is update the docs to say "never use Ref as a struct field"? However this is a bit complicated by the fact that if you use Ref{Int} as a field type, assigning an Int to it will actually work. Assigning a Ptr to a Ref field may be wrong, but it's wrong in a very confusing way that could benefit from an error message.

@jiahao
Copy link
Collaborator

jiahao commented Feb 3, 2016

As I understood from discussion with @JeffBezanson and @vtjnash offline, the general rule of thumb is to use Ref{T} only in covariant context to replace the x = Array(T, 1); ccall(... &x) idiom in old Julia. In other scenarios like the return value of a ccall and a Julia type mimicking a C struct, Ptr{T} should be used instead as the proper representation of a C pointer (an integer representing a potentially useful address, lacking the address validation and GC rooting created by Ref to a Julia object).

jiahao added a commit to JuliaLang/julia that referenced this issue Feb 4, 2016
- Remove references to #2818

- Explain when to use `Ptr{T}` and when to use `Ref{T}` correctly.
  `Ptr` is generally used for return types and fields of types mirroring
  C structs. `Ref` is generally used for input types, allowing memory
  managed by either C or Julia to be referenced by `ccall`.

- Provide some examples of C wrappers simplified from GSL.jl

- Other minor formatting change

Ref: JuliaMath/GSL.jl#43
jiahao added a commit to JuliaLang/julia that referenced this issue Feb 4, 2016
- Remove references to #2818

- Explain when to use `Ptr{T}` and when to use `Ref{T}` correctly.
  `Ptr` is generally used for return types and fields of types mirroring
  C structs. `Ref` is generally used for input types, allowing memory
  managed by either C or Julia to be referenced by `ccall`.

- Provide some examples of C wrappers simplified from GSL.jl

- Other minor formatting change

Ref: JuliaMath/GSL.jl#43
jiahao added a commit to JuliaLang/julia that referenced this issue Feb 4, 2016
- Remove references to #2818

- Explain when to use `Ptr{T}` and when to use `Ref{T}` correctly.
  `Ptr` is generally used for return types and fields of types mirroring
  C structs. `Ref` is generally used for input types, allowing memory
  managed by either C or Julia to be referenced by `ccall`.

- Provide some examples of C wrappers simplified from GSL.jl

- Fix description of `cconvert` in the Auto-conversion section

- Better cross-referencing to the standard library

- Other minor formatting changes

Ref: JuliaMath/GSL.jl#43
jiahao added a commit to JuliaLang/julia that referenced this issue Feb 4, 2016
- Remove references to #2818

- Explain when to use `Ptr{T}` and when to use `Ref{T}` correctly.
  `Ptr` is generally used for return types and fields of types mirroring
  C structs. `Ref` is generally used for input types, allowing memory
  managed by either C or Julia to be referenced by `ccall`.

- Provide some examples of C wrappers simplified from GSL.jl, with
  comments delineating the various parts of the `ccall`.

- Fix description of `cconvert` in the Auto-conversion section

- Better cross-referencing to the standard library

- Other minor formatting changes

Ref: JuliaMath/GSL.jl#43
@jiahao
Copy link
Collaborator

jiahao commented Feb 8, 2016

I think all reasonable actions pertaining to this issue have been taken. The Julia manual has been updated and Refs excised.

@jiahao jiahao closed this as completed Feb 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants