Skip to content

format_strings breaks after "\\" resulting in invalid output #5138

@bddap

Description

@bddap
#[rustfmt::skip]
fn raw() {
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\a";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\";
}

fn formatted() {
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\
     \a";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
     \";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
     \\\";
}

rustfmt 1.4.38-nightly (efec545 2021-12-04)
installed via rustup

Activity

bddap

bddap commented on Dec 14, 2021

@bddap
Author

format_strings = true

Note this happens when the "\\" is at a position close to max width.

added
only-with-optionrequires a non-default option value to reproduce
bugPanic, non-idempotency, invalid code, etc.
on Dec 15, 2021
calebcartwright

calebcartwright commented on Dec 16, 2021

@calebcartwright
Member

This is one I'm fairly confident has been fixed on the other branch and just needs to have the commits cherry-picked over

ytmimi

ytmimi commented on Dec 17, 2021

@ytmimi
Contributor

This is one I'm fairly confident has been fixed on the other branch and just needs to have the commits cherry-picked over

Was it maybe 472a2ed?

calebcartwright

calebcartwright commented on Dec 21, 2021

@calebcartwright
Member

This is one I'm fairly confident has been fixed on the other branch and just needs to have the commits cherry-picked over

Was it maybe 472a2ed?

I actually was thinking about #4524. Whenever you get a chance would you mind seeing if this behavior is reproducible on the 2.0 branch? If not that means the it's covered by that fix too, in which case we should give that another quick pass and consider backporting the commit(s)

ytmimi

ytmimi commented on Dec 23, 2021

@ytmimi
Contributor

on the 2.x branch here's how rustfmt formats the input:

fn raw() {
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\
    \\a";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    \";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    \\\";
}

It's a slightly different formatting that the 1.x branch, but it still doesn't compile. The silver lining here is that the 2.x branch formats the first line in a way that would compile if it was the only line that we needed to be reformatted.

// This compiles
fn main() {
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\
    \\a";
}

The bad news is that the last two lines aren't formatted any differently so there's still some work to do, but I think backporting the existing changes is a good starting point.

calebcartwright

calebcartwright commented on Dec 23, 2021

@calebcartwright
Member

Thanks for digging into this!

The bad news is that the last two lines aren't formatted any differently so there's still some work to do, but I think backporting the existing changes is a good starting point.

Perhaps, though if that doesn't fully address the overall issue it may be better to try to find a holistic solution instead

ytmimi

ytmimi commented on Dec 23, 2021

@ytmimi
Contributor

Perhaps, though if that doesn't fully address the overall issue it may be better to try to find a holistic solution instead

Yeah you might be right

scovl

scovl commented on Jan 26, 2022

@scovl

This incorrect output has already been fixed in version 1.60.0-nightly. In fact, the following output appears:

error[internal]: line formatted, but exceeded maximum width (maximum: 100 (see max_width option), found: 103)

In this case, I don't know if the correct thing would be to increase the size or simply show the same message. Also because the case above, from a practical and functional point of view, does not seem to make sense to me. Any objection to this question @bddap ?

// This compiles
fn main() {
    println!(
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\
    \\a"
    );
}
bddap

bddap commented on Jan 26, 2022

@bddap
Author

Sorry, @lobocode . Not sure I understand what you are asking.

scovl

scovl commented on Jan 27, 2022

@scovl

Sorry, @lobocode . Not sure I understand what you are asking.

I'm asking if you've checked, if this bug has been fixed in later versions of rustfmt.

bddap

bddap commented on Jan 31, 2022

@bddap
Author

I have not.

added 2 commits that reference this issue on Aug 4, 2022
2474b49
12e16ba
ldm0

ldm0 commented on Oct 8, 2022

@ldm0
fn main() {
    let body = format!("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"");
    
    /* After cargo fmt:
    
    let body = format!(
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\
         ""
    );
    
     */
}

Another badcase with format_strings = true enabled.

added
1x-backport:pendingFixed/resolved in source but not yet backported to a 1x branch and release
on Oct 12, 2022
added a commit that references this issue on Dec 7, 2023
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

    1x-backport:pendingFixed/resolved in source but not yet backported to a 1x branch and releasea-stringsString literalsbugPanic, non-idempotency, invalid code, etc.help wantedonly-with-optionrequires a non-default option value to reproduce

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @scovl@bddap@calebcartwright@ytmimi@ldm0

      Issue actions

        format_strings breaks after "\\" resulting in invalid output · Issue #5138 · rust-lang/rustfmt