Rename various query cycle things.#153979
Conversation
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.
|
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. |
|
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. |
|
☔ The latest upstream changes (presumably #153984) made this pull request unmergeable. Please resolve the merge conflicts. |
Not for the parallel case though, it's a list of edges labeled with |
Suggestion for a better name for that field? |
|
|
|
But if |
The existing names have bugged me for a while. Changes:
CycleError->Cycle. Because we normally use "error" for aDiagwithLevel::Error, and this type is just a precursor to that. Also, many existing locals of this type are already namedcycle.CycleError::cycle->Cycle::stack. Because it is a stack. And a couple of existing locals are already namedstack.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. theDiag).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_fnsuffix is for consistency with other vtable fns.from_cycle_error->handle_cycle_error. A similar story for this module.