types_escaping_snapshot cleanup finalization #84570

Activity

1. Mark-Simulacrum

   added

   C-cleanupCategory: PRs that clean code up or issues documenting cleanup.Category: PRs that clean code up or issues documenting cleanup.

   T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.

   on Apr 26, 2021
2. jyn514 commented on Apr 26, 2021

   

   jyn514

   on Apr 26, 2021

   Last edited by jyn514

   Member

   More actions

   I'm not sure what's going on with that code. The comment on impl sv::SnapshotVecDelegate for Delegate { says

   // In fact, we could almost just remove the
   // SnapshotVec entirely, except that we would have to
   // reproduce some of its logic, since we want to know which
   // type variables have been instantiated since the snapshot
   // was started, so we can implement types_escaping_snapshot.

   but when I try to remove the impl I get lots of errors:

   error[E0277]: the trait bound `type_variable::Delegate: SnapshotVecDelegate` is not satisfied
      --> compiler/rustc_infer/src/infer/type_variable.rs:319:10
       |
   319 |     ) -> sv::SnapshotVec<Delegate, &mut Vec<TypeVariableData>, &mut InferCtxtUndoLogs<'tcx>> {
       |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `SnapshotVecDelegate` is not implemented for `type_variable::Delegate`
       | 
      ::: /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/ena-0.14.0/src/snapshot_vec.rs:111:8
       |
   111 |     D: SnapshotVecDelegate,
       |        ------------------- required by this bound in `SnapshotVec`
   

   including from functions that are still used, like values().

   The comment was added in 57a593f - @nikomatsakis do any obvious cleanups come to mind now that types_escaping_snapshot no longer exists?
3. jyn514 commented on Apr 26, 2021

   

   jyn514

   on Apr 26, 2021

   Last edited by jyn514

   Member

   More actions

   I did remove

   rust/compiler/rustc_infer/src/infer/type_variable.rs

   Line 227 in 6bed7e1

   self.undo_log.push(Instantiate { vid });

   without any test failures. Maybe self.undo_log can be removed altogether?
4. nikomatsakis commented on Apr 27, 2021

   

   nikomatsakis

   on Apr 27, 2021

   Contributor

   More actions

   Hmm
5. nikomatsakis commented on Apr 27, 2021

   

   nikomatsakis

   on Apr 27, 2021

   Contributor

   More actions

   I don't think you can remove the undo_log, that's needed for probes and other things, but I might be forgetting something. There might be some simplifications possible.