Skip to content

Style edition 2024: overflow_delimited_expr behavior should be reconsidered before stabilization #136224

Closed
@kpreid

Description

@kpreid

(The following is a continuation of this Zulip thread.)

Style edition 2024, as implemented in rustfmt via implying the option overflow_delimited_expr = true, causes certain code to be wrapped very oddly. While this formatting is “working as designed”, I believe it should be reconsidered — at least its stabilization in its current form in the 2024 edition. It will affect any code which uses nested arrays, particularly to write literal tables/matrices. Example from my code:

// 2021
const FACE_TABLE: [[Face7; 2]; 3] = [
    [Face7::PX, Face7::NX],
    [Face7::PY, Face7::NY],
    [Face7::PZ, Face7::NZ],
];

// 2024
const FACE_TABLE: [[Face7; 2]; 3] = [[Face7::PX, Face7::NX], [Face7::PY, Face7::NY], [
    Face7::PZ,
    Face7::NZ,
]];

It will also affect any function or macro call which accepts two semantically parallel arrays, structs, or literals. This is more debatable, but I think it is often worse:

// 2021
let bounds = GridAab::from_lower_upper(
    [-2, -1, -2],
    [(n_x - 1) * spacing_x + 3, 20, (n_z - 1) * spacing_z + 3],
);

// 2024
let bounds = GridAab::from_lower_upper([-2, -1, -2], [
    (n_x - 1) * spacing_x + 3,
    20,
    (n_z - 1) * spacing_z + 3,
]);
// 2021
assert_eq!(
    Rgba::new(0.125, 0.25, 0.5, 0.75).to_srgb8(),
    [99, 137, 188, 191]
);

// 2024
assert_eq!(Rgba::new(0.125, 0.25, 0.5, 0.75).to_srgb8(), [
    99, 137, 188, 191
]);

See rust-lang/rustfmt#6452 for more examples of bad formatting from someone else’s code.

Some codebases will be greatly affected by this because they have large numbers of such calls — depending on their problem domain and API design. It is possible to prevent rustfmt from changing to overflow style by adding a strategic comment:

const FACE_TABLE: [[Face7; 2]; 3] = [
    [Face7::PX, Face7::NX], //
    [Face7::PY, Face7::NY],
    [Face7::PZ, Face7::NZ],
];

but this is an unobvious trick, and applying it to existing code is an unpleasant experience to have as part of edition migration.

I'm not saying that this is a bad change overall. In fact, I am very pleased by its reduction in indentation levels and vertical space occupied solely by brackets, which I think will aid readability by allowing more code to fit on screen at once. However, I think that in its current form, its effect on arrays and other parallel constructs is a significant regression in Rust’s quality of “having good default behavior”, which is going to affect a lot of people who choose to attempt 2024 edition migration as soon as Rust 1.85 is released.

Therefore, I believe that the stabilization of the 2024 style edition with overflow_delimited_expr=true behavior, in its current form and without also stabilizing the ability to control overflow_delimited_expr, is unwise. Comments on the PR adding this style change suggested that it would be desirable to stabilize the option too, but no action has currently been taken. I’m filing this issue in order to bring attention to the matter and provide an opportunity to discuss whether it is wise to stabilize this formatting in its current form.

I believe that one of the following actions should be taken:

  • Remove overflow_delimited_expr=true from the 2024 style edition, deferring it until it can be revised. (Least-effort option.)
  • Change overflow_delimited_expr=true so that it does not do this.
    • Perhaps it does not overflow a final array if the preceding element is also an array.
    • ia0 had another scheme.
  • Stabilize the overflow_delimited_expr rustfmt option so that negatively affected codebases can disable it.

Per TC's request, nominating this for discussion:

@rustbot label +A-edition-2024 +T-style +T-rustfmt +I-style-nominated

CC @traviscross @ytmimi @pitaj @rust-lang/style @rust-lang/rustfmt @ehuss

Tracking:

Alternate rule proposal:

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-edition-2024Area: The 2024 editionA-rustfmtArea: RustfmtI-style-nominatedNominated for discussion during a style team meeting.T-styleRelevant to the style 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

    Issue actions