Skip to content

Backslash incorrectly removed. #716

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mAarnos opened this issue Dec 22, 2015 · 10 comments
Closed

Backslash incorrectly removed. #716

mAarnos opened this issue Dec 22, 2015 · 10 comments
Labels
bug Panic, non-idempotency, invalid code, etc. duplicate only-with-option requires a non-default option value to reproduce

Comments

@mAarnos
Copy link

mAarnos commented Dec 22, 2015

The code

println!("forall x. forall y. forall z. mult(x, mult(y, z)) = mult(mult(x, y), z) /\\
          forall x. mult(e(), x) = x /\\
          forall x. mult(x, x) = e()
          ==> forall x. forall y. mult(x, y) = mult(y, x)");

gets formatted to

println!("forall x. forall y. forall z. mult(x, mult(y, z)) = mult(mult(x, y), z) /\forall \
          x. mult(e(), x) = x /\forall x. mult(x, x) = e()
          ==> forall x. \
          forall y. mult(x, y) = mult(y, x)");

which is clearly incorrect as it is no longer possible to compile the code. Also, the formatting is absolutely horrible but that is a separate issue. This happens with the latest development version (compiled with 1.5 stable), and the version installed with 'cargo install rustfmt' (again, compiled with 1.5 stable). I am using Windows 7 but I don't believe that is relevant.

@nrc nrc added the bug Panic, non-idempotency, invalid code, etc. label Dec 22, 2015
@nrc
Copy link
Member

nrc commented Dec 22, 2015

Other than fixing the backslash bug, I wonder if we could do better at preserving formatting in string literals - seems to be a common problem that people have significant spacing which rustfmt then stomps all over.

@nrc
Copy link
Member

nrc commented Apr 6, 2016

huh, this triggers an ICE.

I suspect once the ICE is fixed, this will be fixed by #726.

@nrc nrc added this to the 1.0 milestone Apr 6, 2016
@nrc
Copy link
Member

nrc commented Apr 6, 2016

No ICE actually, that was something else.

It seems that we now insert some extra slashes:

if expando.ptr.is_null() {
        println!("forall x. forall y. forall z. mult(x, mult(y, z)) = mult(mult(x, y), z) /\\
              \
                  forall x. mult(e(), x) = x /\\
              forall x. mult(x, x) = e()
              \
                  ==> forall x. forall y. mult(x, y) = mult(y, x)");
    }

@nrc nrc removed this from the 1.0 milestone Apr 6, 2016
@drusellers
Copy link

Other than fixing the backslash bug, I wonder if we could do better at preserving formatting in string literals - seems to be a common problem that people have significant spacing which rustfmt then stomps all over.

Is there an issue for this? rustfmt is mangling my inline SQL statements.

@nrc
Copy link
Member

nrc commented Jan 26, 2017

Well, we don't muck up the formatting any more, but we do still remove the slash, which is weird. Investigating...

@nrc
Copy link
Member

nrc commented Jan 26, 2017

No actually, that is a lie, we totally leave it alone

@nrc nrc added the only-with-option requires a non-default option value to reproduce label Jan 26, 2017
@davidalber
Copy link
Contributor

Here's how it is formatted today (0.2.15-nightly) with default configuration:

println!(
    "forall x. forall y. forall z. mult(x, mult(y, z)) = mult(mult(x, y), z) /\\
          forall x. mult(e(), x) = x /\\
          forall x. mult(x, x) = e()
          ==> forall x. forall y. mult(x, y) = mult(y, x)"
);

Diff from original:

Diff in /Users/davidalber/dev/foo/716.rs at line 1:
-println!("forall x. forall y. forall z. mult(x, mult(y, z)) = mult(mult(x, y), z) /\\⏎
+println!(⏎
+    "forall x. forall y. forall z. mult(x, mult(y, z)) = mult(mult(x, y), z) /\\⏎
           forall x. mult(e(), x) = x /\\⏎
           forall x. mult(x, x) = e()⏎
-          ==> forall x. forall y. mult(x, y) = mult(y, x)");⏎
+          ==> forall x. forall y. mult(x, y) = mult(y, x)"⏎
+);⏎

If I put it inside a function the spacing turns out a little differently, but it otherwise the same.

fn main() {
    println!(
        "forall x. forall y. forall z. mult(x, mult(y, z)) = mult(mult(x, y), z) /\\
          forall x. mult(e(), x) = x /\\
          forall x. mult(x, x) = e()
          ==> forall x. forall y. mult(x, y) = mult(y, x)"
    );
}

@nrc: does the "only-with-option" label mean that this requires a non-default configuration value to repro a problem? If yes, what is it?

@nrc nrc added only-with-option requires a non-default option value to reproduce and removed only-with-option requires a non-default option value to reproduce labels Nov 19, 2017
@nrc
Copy link
Member

nrc commented Nov 19, 2017

I assumed that the option to re-format string literals (format_strings and/or force_format_strings) was required since otherwise we should not touch string literals at all.

@davidalber
Copy link
Contributor

I see. With some combinations of format_strings and max_width I can get extra backslashes. Here's a rustfmt.toml:

format_strings = true
max_width = 50

Then the example from my last comment formats to the following.

println!(
    "forall x. forall y. forall z. mult(x, \
     mult(y, z)) = mult(mult(x, y), z) /\\
          \
     forall x. mult(e(), x) = x /\\
          \
     forall x. mult(x, x) = e()
          ==> \
     forall x. forall y. mult(x, y) = mult(y, x)"
);

This seems like the issue also described in #1210.

Is the extra backslash the only remaining issue in here? My reading of the thread is that the original issue (removal of a backslash) is resolved. Have I got that right?

@nrc nrc added the duplicate label Nov 20, 2017
@nrc
Copy link
Member

nrc commented Nov 20, 2017

@davidalber thanks for investigating this! Yes it seems like the original issue is resolved and what is left is a dup of #1210. So, lets close this as a dup.

@nrc nrc closed this as completed Nov 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Panic, non-idempotency, invalid code, etc. duplicate only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

No branches or pull requests

5 participants