Skip to content

Rename various query cycle things.#153979

Draft
nnethercote wants to merge 1 commit intorust-lang:mainfrom
nnethercote:rename-cycle-things
Draft

Rename various query cycle things.#153979
nnethercote wants to merge 1 commit intorust-lang:mainfrom
nnethercote:rename-cycle-things

Conversation

@nnethercote
Copy link
Contributor

The existing names have bugged me for a while. Changes:

  • CycleError -> Cycle. Because we normally use "error" for a Diag with Level::Error, and this type is just a precursor to that. Also, many existing locals of this type are already named cycle.

  • CycleError::cycle -> Cycle::stack. Because it is a stack. And a couple of existing locals are already named stack.

  • cycle_error -> find_and_handle_cycle. Because that's what it does. The existing name is just a non-descript noun phrase.

  • mk_cycle -> handle_cycle. Because it doesn't make the cycle; the cycle is already made. It handles the cycle, which involves (a) creating the error, and (b) handling the error.

  • report_cycle -> create_cycle_error. Because that's what it does: creates the cycle error (i.e. the Diag).

  • value_from_cycle_error -> handle_cycle_error_fn. Because most cases don't produce a value, they just emit an error and quit. And the _fn suffix is for consistency with other vtable fns.

  • from_cycle_error -> handle_cycle_error. A similar story for this module.

The existing names have bugged me for a while. Changes:

- `CycleError` -> `Cycle`. Because we normally use "error" for a `Diag`
  with `Level::Error`, and this type is just a precursor to that. Also,
  many existing locals of this type are already named `cycle`.

- `CycleError::cycle` -> `Cycle::stack`. Because it is a stack. And a
  couple of existing locals are already named `stack`.

- `cycle_error` -> `find_and_handle_cycle`. Because that's what it does.
  The existing name is just a non-descript noun phrase.

- `mk_cycle` -> `handle_cycle`. Because it doesn't make the cycle; the
  cycle is already made. It handles the cycle, which involves (a)
  creating the error, and (b) handling the error.

- `report_cycle` -> `create_cycle_error`. Because that's what it does:
  creates the cycle error (i.e. the `Diag`).

- `value_from_cycle_error` -> `handle_cycle_error_fn`. Because most
  cases don't produce a value, they just emit an error and quit.
  And the `_fn` suffix is for consistency with other vtable fns.

- `from_cycle_error` -> `handle_cycle_error`. A similar story for this
  module.
@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 17, 2026
@nnethercote
Copy link
Contributor Author

cc @Zalathar @Zoxc @zetanumbers

I know this has high conflict potential so I'm happy to wait on merging this if it will cause problems for any other current work. But I did want to see what it would look like. I think it turned out well.

@zetanumbers
Copy link
Contributor

zetanumbers commented Mar 17, 2026

No, I don't like renamings. I have to adjust every time. Thanks for notifying beforehand at least.

But about the matter. The word "handle" seems like one of those fit-everything words like "data", etc.

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 17, 2026

☔ The latest upstream changes (presumably #153984) made this pull request unmergeable. Please resolve the merge conflicts.

@Zoxc
Copy link
Contributor

Zoxc commented Mar 17, 2026

  • CycleError::cycle -> Cycle::stack. Because it is a stack

Not for the parallel case though, it's a list of edges labeled with Span forming a cycle.

@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 17, 2026

Not for the parallel case though, it's a list of edges labeled with Span forming a cycle.

Suggestion for a better name for that field? frames?

@Zoxc
Copy link
Contributor

Zoxc commented Mar 19, 2026

cycle does fit quite well.

@nnethercote
Copy link
Contributor Author

But if Cycle::cycle is a field then we end up writing cycle.cycle a lot, which feels bad. cycle.frames seems better to me.

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

Labels

A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants