-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
extend api
#3802
base: trunk
Are you sure you want to change the base?
extend api
#3802
Conversation
proposals/p3802.md
Outdated
declaration, so these lookups into `T` can succeed. | ||
|
||
The general rule for resolving ambiguity for `extend`, which we apply here as | ||
well, is that if lookup into `Box(T)` succeeds, then that result is used and no |
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.
This would be easier to follow if it appeared after the Box
example below or had a link to that example.
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 rewrote it a bit, but let me know if it would be better to move this until after the Box
example.
proposals/p3802.md
Outdated
return {.ptr = self.Op(p->ptr)}; | ||
} | ||
} | ||
// (Plus impls for the other three combinations of ValueBind and RefBind.) |
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.
Exploring these myself:
// <Ref(T) value>.<ref bindable> produces a Ref(R) value expression.
impl forall [T:! type, U:! RefBind(T)]
U as ValueBind(Ref(T)) where .Result = Ref(U.Result) {
fn Op[self: Self](p: Ref(T)) -> Result {
return {.ptr = self.Op(p->ptr)};
}
}
// <Ref(T) ref expr>.<ref bindable> produces a reference expression.
// Reference collapse: no added Ref(...) here.
impl forall [T:! type, U:! RefBind(T)]
U as RefBind(Ref(T)) where .Result = U.Result {
fn Op[self: Self](p: Ref(T)*) -> Result* {
return self.Op(p->ptr);
}
}
// <Ref(T) value>.<value bindable> produces a value expression.
// A value binding is performed to turn the Ref(T) into a T.
impl forall [T:! type, U:! ValueBind(T)]
U as ValueBind(Ref(T)) where .Result = U.Result {
fn Op[self: Self](p: Ref(T)) -> Result {
return self.Op(*p.ptr);
}
}
I think implementing U as RefBind(Ref(T))
in terms of U as ValueBind(T)
is not possible. We can't produce a pointer as a result of the binding.
There's also overlap between the first and third impl
s. And the first and second violate the new coherence rule in #3720: given ref: Ref({.n: i32})
, ref.n
will either be a value expression of type Ref(i32)
or a reference expression of type i32
depending on the expression category of ref
.
I think maybe the right thing to do is to drop the second impl
entirely, and have only two impl
s, both for ValueBind
, with a match_first
:
match_first {
// <Ref(T) value>.<ref bindable> produces a value expression of type `Ref(R)`.
impl forall [T:! type, U:! RefBind(T)]
U as ValueBind(Ref(T)) where .Result = Ref(U.Result) {
fn Op[self: Self](p: Ref(T)) -> Result {
return {.ptr = self.Op(p->ptr)};
}
}
// <Ref(T) value>.<value bindable> produces a value expression of type `R`.
// A value binding is performed to turn the Ref(T) into a T.
impl forall [T:! type, U:! ValueBind(T)]
U as ValueBind(Ref(T)) where .Result = U.Result {
fn Op[self: Self](p: Ref(T)) -> Result {
return self.Op(*p.ptr);
}
}
}
For this to really work, I think we'd need binding to fall back from RefBind
to ValueBind
if reference binding isn't possible, as discussed in #3720. The above pair of impl
s would then be implementing the same fallback semantics for Ref(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.
I don't think this works. #3720 only ever produces a reference expression (by automatically dereferencing the returned pointer) when the expression on the left side of the .
is a reference expression. I think this makes Ref(T)
impossible, and I should probably delete this section.
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.
Sorry, I had not understood what you were proposing (that you can't get a reference expression, but can get a Ref(T)
value that acts like one). I've made these changes, added the additional support for calls on Ref(T)
values, and moved this section to future work, as discussed in open discussion on 05-16.
This application of `extend api` and binding was first described in | ||
[this comment on #3720](https://github.com/carbon-language/carbon-lang/pull/3720/files/37e967ff53e89e345eecb49ec79c6dfbe18a3c54#r1513712572). | ||
|
||
### Use case: with implicit conversion |
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.
Note to self: I'm parking this review here while looking at other things.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
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.
Should be ready for another look now.
proposals/p3802.md
Outdated
return {.ptr = self.Op(p->ptr)}; | ||
} | ||
} | ||
// (Plus impls for the other three combinations of ValueBind and RefBind.) |
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.
Sorry, I had not understood what you were proposing (that you can't get a reference expression, but can get a Ref(T)
value that acts like one). I've made these changes, added the additional support for calls on Ref(T)
values, and moved this section to future work, as discussed in open discussion on 05-16.
proposals/p3802.md
Outdated
declaration, so these lookups into `T` can succeed. | ||
|
||
The general rule for resolving ambiguity for `extend`, which we apply here as | ||
well, is that if lookup into `Box(T)` succeeds, then that result is used and no |
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 rewrote it a bit, but let me know if it would be better to move this until after the Box
example.
We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please comment or remove the This PR is labeled |
@josh11b and I have both lost track of our state on this. We intend to return to this at some point but this PR is not a current priority. |
Allow types to
extend api
other types, adding the names from the other type to its namespace, for forwarding and delegation use cases.