Skip to content

types_escaping_snapshot cleanup finalization #84570

Open
@Mark-Simulacrum

Description

@Mark-Simulacrum
Member

#83185 deleted types_escaping_snapshot in compiler/rustc_infer/src/infer/type_variable.rs, but there are several comments remaining referencing that function, some of which seem to indicate no longer necessary operations. This code, to my knowledge, is pretty finicky, so we will want to be careful, but there may be some performance (or at least cleanup) wins left there.

cc @jyn514

Activity

added
C-cleanupCategory: PRs that clean code up or issues documenting cleanup.
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
on Apr 26, 2021
jyn514

jyn514 commented on Apr 26, 2021

@jyn514
Member

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?

jyn514

jyn514 commented on Apr 26, 2021

@jyn514
Member

I did remove

self.undo_log.push(Instantiate { vid });
without any test failures. Maybe self.undo_log can be removed altogether?

nikomatsakis

nikomatsakis commented on Apr 27, 2021

@nikomatsakis
Contributor

Hmm

nikomatsakis

nikomatsakis commented on Apr 27, 2021

@nikomatsakis
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-cleanupCategory: PRs that clean code up or issues documenting cleanup.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @nikomatsakis@Mark-Simulacrum@jyn514

        Issue actions

          types_escaping_snapshot cleanup finalization · Issue #84570 · rust-lang/rust