add dynamic for smir #113969
add dynamic for smir #113969
Conversation
rustbot
added
S-waiting-on-review
|
☔ The latest upstream changes (presumably #113943) made this pull request unmergeable. Please resolve the merge conflicts. |
| type T = stable_mir::ty::PolyFnSig; | ||
| fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { | ||
| use stable_mir::ty::Binder; | ||
| impl<'tcx, S, T> Stable<'tcx> for ty::Binder<'tcx, S> |
There was a problem hiding this comment.
Can you rename the T here to V. I think it will be clearer because we won't have the T param and the T associated type and won't have this T = T thing that looks weird :).
| use stable_mir::ty::Binder; | ||
| impl<'tcx, S, T> Stable<'tcx> for ty::Binder<'tcx, S> | ||
| where | ||
| S: Stable<'tcx, T = T> + Copy, |
There was a problem hiding this comment.
Unsure what's the preferred way here, if having Copy as bound, if using Deref and calling as_deref() and then implement stable for what's returned or if calling clone.
cc @oli-obk
There was a problem hiding this comment.
I considered using a Clone bound but I thought it might be too weak?
If there's a type with an expensive clone, I'm not sure I wanted to implicitly derive an expensive implementation of stable
There was a problem hiding this comment.
Also, isn't as_deref just an Option thing? I thought the method you get from Deref is just deref
There was a problem hiding this comment.
If I'm not wrong, this works ...
diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs
index dd0d7df9ecf..b7e827dced0 100644
--- a/compiler/rustc_smir/src/rustc_smir/mod.rs
+++ b/compiler/rustc_smir/src/rustc_smir/mod.rs
@@ -581,13 +581,13 @@ fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T {
impl<'tcx, S, T> Stable<'tcx> for ty::Binder<'tcx, S>
where
- S: Stable<'tcx, T = T> + Copy,
+ S: Stable<'tcx, T = T>,
{
type T = stable_mir::ty::Binder<T>;
fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T {
stable_mir::ty::Binder {
- value: self.skip_binder().stable(tables),
+ value: self.as_ref().skip_binder().stable(tables),
bound_vars: self
.bound_vars()
.iter()There was a problem hiding this comment.
Oh as_ref not as_deref
There was a problem hiding this comment.
Oh ignore me I'm being dumb--binder has both
|
This looks good to me, please squash the commits. Maybe you can make a first commit with the Binder change and then the one that adds Dynamic. |
|
Sure I can squash as such. I'm actually not done though, hence the PR being a draft. In particular, I think I want to handle |
ericmarkmartin
force-pushed
the
smir-ty-dynamic
branch
3 times, most recently
from
4172327 to
c5ab118
Compare
ericmarkmartin
force-pushed
the
smir-ty-dynamic
branch
from
c5ab118 to
7010d80
Compare
ericmarkmartin
force-pushed
the
smir-ty-dynamic
branch
from
7010d80 to
badb617
Compare
|
This PR changes Stable MIR cc @oli-obk, @celinval, @spastorino |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#113969 (add dynamic for smir) - rust-lang#113985 (Use erased self type when autoderefing for trait error suggestion) - rust-lang#113987 (Comment stuff in the new solver) - rust-lang#113992 (arm-none fixups) - rust-lang#113993 (Optimize format usage) - rust-lang#113994 (Optimize format usage) - rust-lang#114006 (Update sparc-unknown-none-elf platform README) - rust-lang#114021 (Add missing documentation for `Session::time`) r? `@ghost` `@rustbot` modify labels: rollup
r? spastorino