Introduce marker types #11768
Introduce marker types #11768
Conversation
| /// unsafe { | ||
| /// let x: *T = cast::transmute(s.x); | ||
| /// *x | ||
| /// } |
There was a problem hiding this comment.
I think this is missing a }
|
There's a lot about this I don't understand. Maybe you can explain it to me offline. |
|
This looks really awesome, nice work! |
| /// the type parameter `T` is irrelevant, and hence a `S<int>` can | ||
| /// safely be coerced to a `S<~[int]>` (or, for that matter, `S<U>` for | ||
| /// for any `U`). But this is incorrect because `get()` converts the | ||
| /// `*()` into a `fn(T)` and then passes a value of type `T` to it. |
There was a problem hiding this comment.
The type signature in the example suggests it converts it into a fn(T) -> T, should the return value above be ()?
There was a problem hiding this comment.
Yes, corrected.
|
Note: not all tests pass yet. |
|
|
||
| /// A mutable memory location that admits only `Pod` data. | ||
| #[no_freeze] | ||
| #[deriving(Clone)] |
There was a problem hiding this comment.
This is failing with:
/home/flaper87/workspace/personal/rust/src/libstd/cell.rs:18:12: 18:17 error: mismatched types: expected `std::kinds::marker::InvariantType<T>` but found `&std::kinds::marker::InvariantType<T>` (expected struct st
d::kinds::marker::InvariantType but found &-ptr)
/home/flaper87/workspace/personal/rust/src/libstd/cell.rs:18 #[deriving(Clone)]
^~~~~
/home/flaper87/workspace/personal/rust/src/libstd/cell.rs:18:12: 18:17 error: mismatched types: expected `std::kinds::marker::NoFreeze` but found `&std::kinds::marker::NoFreeze` (expected struct std::kinds::marker
::NoFreeze but found &-ptr)
/home/flaper87/workspace/personal/rust/src/libstd/cell.rs:18 #[deriving(Clone)]
^~~~~
There was a problem hiding this comment.
This is probably caused because NoFreeze and InvariantType don't implement Clone.
There was a problem hiding this comment.
This is a common deriving(Clone) error, so I'm proposing a lint: #11818
There was a problem hiding this comment.
As it happens, it is completely wrong for Cell to derive clone. I am not quite sure why this was failing, though, I think maybe something to do with the weird setup of tests and libstd? In any case, I have fixed the error locally and will push an updated version.
|
@nikomatsakis you said "not all tests pass yet"; will you post here when they do? Anyway, I'll start a review and provide what feedback I can. |
| @@ -1237,17 +1237,18 @@ pub fn make_return_pointer(fcx: &FunctionContext, output_type: ty::t) | |||
| // | |||
| // Be warned! You must call `init_function` before doing anything with the | |||
There was a problem hiding this comment.
you removed the above warning about init_function causing segfaults. Is your plan to later put in code to call init_function from within new_fn_ctxt, but you didn't want to remove this comment here about init_function until doing that?
Or does your code cause a hypothetical addition of init_function to no longer type-check, which is my more recent inference from reading your comment here? (Which would mean that this comment should indeed stay while the other comment should not stay, at least not in its current form.)
There was a problem hiding this comment.
I don't believe there is any danger. I didn't merge the two functions simply because that would have added yet more parameters to new_fn_ctxt, but maybe I should do so. It's not like one more parameter will hurt.
|
Tests pass now, addressed previous comments. |
|
grr failed to commit my latest edits before pushing. |
|
@pnkfelix I never wound up writing the table you suggested; it makes sense though, maybe we can add it or open an issue or something. I guess there's a limit to the extent that we can explain variance in a doc comment...I was hoping we could at least identify common scenarios (e.g., mutability) and suggest to people which marker to use, in lieu of a deeper understanding (that's what I was going for, not sure if I succeeded). |
|
Along those lines, I basically think people ought to use invariance whenever in doubt. |
|
I have no idea what those failures are. I can't reproduce locally yet :( |
|
I fix a thing or two with the bots recently, so these may have just been sporadic failures (I can't find the exact logs on this PR any more) |
|
Well, I'll give it one more try. If I see those failures again we'll have to dig deeper in. |
of builtin bounds. Fixes rust-lang#10834. Fixes rust-lang#11385. cc rust-lang#5922.
|
w000t it landed! |
Introduce marker types for indicating variance and for opting out
of builtin bounds.
Fixes #10834.
Fixes #11385.
cc #5922.
r? @pnkfelix (since you reviewed the variance inference in the first place)