Skip to content
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

Don't drop blocks on foreign functions #4314

Merged
merged 1 commit into from
Jul 14, 2020

Conversation

ayazhafiz
Copy link
Contributor

A code like

extern "C" {
    fn f() {
        fn g() {}
    }
}

is incorrect and does not compile. Today rustfmt formats this in a way
that is correct:

extern "C" {
    fn f();
}

But this loses information, and doesn't have to be done because we know
the content of the block if it is present. During development I don't
think rustfmt should drop the block in this context.

Closes #4313

A code like

```rust
extern "C" {
    fn f() {
        fn g() {}
    }
}
```

is incorrect and does not compile. Today rustfmt formats this in a way
that is correct:

```rust
extern "C" {
    fn f();
}
```

But this loses information, and doesn't have to be done because we know
the content of the block if it is present. During development I don't
think rustfmt should drop the block in this context.

Closes rust-lang#4313
@calebcartwright
Copy link
Member

My assumption is that this was an unintentional side effect vs. rustfmt deliberately attempting to convert the input to compilable code so I'm a 👍 on this change but will defer to @topecongiro given the nature of the change.

This may also be something we'd want to sneak into the 2.0 release given the "change" in formatting.

Copy link
Contributor

@topecongiro topecongiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. As you mention, rustfmt should not drop the input. This behavior could be a problem when formatting macros, in particular.

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

Successfully merging this pull request may close these issues.

Foreign function blocks dropped
4 participants