-
Notifications
You must be signed in to change notification settings - Fork 45
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
Allow Relation<Key>
to join with Variable<(Key, Value)>
directly
#36
Conversation
b0edadb
to
675b629
Compare
Whoops, 25c7774 is actually a terrible idea. Now when a |
675b629
to
3ff4ad0
Compare
This looks good to me. And with it, rust-lang/polonius#178 should actually improve on I do wonder what to do about Thankfully these |
Yep! I'm glad I tested that PR first XD. I think we can just refactor the helper function so it works on a |
This case occurred in `Polonius`, where we had to create a separate `Relation<(T, ())>` to make things work. Implementing this required a change to the interface of `JoinInput`, since we are not allowed to assume that a `Relation<T>` and a `Relation<(T, ())>` are layout compatible. Now, `stable` takes a callback instead of returning a slice directly.
Layout compatibility guarantees for `T` do not extend to types containing `T` (like `&[T]`), so use `from_raw_parts` instead of `mem::transmute`.
68605a2
to
586043f
Compare
@ecstatic-morse btw do you know whether miri complained with the previous |
It won't complain on either of them. From the Miri docs:
|
Let's merge this, then. |
Polonius has to create new intermediate variables with types like
Variable<(Key, ())>
fromRelation<Key>
so that they can be joined with variables likeVariable<(Key, Value)>
. This happens once in the naive variant (see below) and twice in the optimized one. This isn't actually necessary, since we can legally transmute aRelation<Key>
to aRelation<(Key, ())>
according to the unsafe-code guidelines. This saves a bit of memory and some time copying tuples around, although these relations aren't very large on any of the inputs bundled with Polonius. Mostly, it makes the code clearer by removing the number of variables used solely for re-indexing.To do this safely, I had to change howRelation
implementsJoinInput
. This actually fixes a bug inRelation::from_antijoin
(which we don't use), but it's not obviously correct. See the second commit for details.I changed the signature of
JoinInput
so its semantics could remain the same.Relation::from_antijoin
is still broken, however.