rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.9k stars 1.56k forks source link

Function body blocks #3629

Open joshtriplett opened 4 months ago

joshtriplett commented 4 months ago

Co-authored-by: Eric Holk

Rendered

kennytm commented 4 months ago

is this supposed to be read together with #3628?

joshtriplett commented 4 months ago

Both have value independently, and one could be approved without the other, but yes, the two proposals both make each other better.

davidbarsky commented 4 months ago

A downside of this proposal that I don't see mentioned is that it would make error recovery more difficult in IDEs like rust-analyzer or RustRover. Delimiters like parentheses/curly brackets make it trivial for IDEs to assume boundaries between snippets of incomplete code, but this RFC would throw a wrench into such approaches. I don't think the aesthetic wins outweigh the tooling downsides, especially for new Rust programmers.

VitWW commented 4 months ago

It will be ok for full "block" expressions:

kennytm commented 4 months ago

i think this will cause some longer-than-expected sequences before disambiguation can be resolved:

fn g<T>() where T: for

the for here may be the looping expression or the HRTB quantifier

fn g<T>() where T: for _ in [0] {}
fn g<T>() where T: for<'a> Fn(&'a u8) {}

(with rust-lang/rust#86935 you have to consume the < as well before determining whether it is HRTB or expression.)

I suspect that with #3628 that async T being a type would cause some parsing issue in the where clause against async {} block too.

joshtriplett commented 4 months ago

@davidbarsky wrote:

A downside of this proposal that I don't see mentioned is that it would make error recovery more difficult in IDEs like rust-analyzer or RustRover. Delimiters like parentheses/curly brackets make it trivial for IDEs to assume boundaries between snippets of incomplete code, but this RFC would throw a wrench into such approaches.

This (for both humans and tools) is exactly why I'm proposing to only allow this for constructs that accept a single block. Allowing an arbitrary expression would have the problem you're describing. Allowing e.g. try { ... } or gen { ... } doesn't seem like it would be substantially more complex than the existing { ... }.

joshtriplett commented 4 months ago

@kennytm wrote:

i think this will cause some longer-than-expected sequences before disambiguation can be resolved:

fn g<T>() where T: for

the for here may be the looping expression or the HRTB quantifier

fn g<T>() where T: for _ in [0] {}
fn g<T>() where T: for<'a> Fn(&'a u8) {}

(with rust-lang/rust#86935 you have to consume the < as well before determining whether it is HRTB or expression.)

In this case, the parser should know that it's expecting a type rather than a function body, right?

And even if not, as you said, after a couple of tokens you know what you have.

I suspect that with #3628 that async T being a type would cause some parsing issue in the where clause against async {} block too.

AFAICT you would know one symbol after async, the moment you see {. Also, I would think that the parser state should know whether it's expecting a type or the possible start of a function, but perhaps there's an ambiguous case.

kennytm commented 4 months ago

@joshtriplett

In this case, the parser should know that it's expecting a type rather than a function body, right?

No, T: is a perfectly fine WhereClauseItem (it just asserts T is well-formed). The code below runs fine today.

fn g<T>() where T: {
    for _ in [0] {}
}

Similar issue with empty where clause:

fn g<T>() where   for<'a> fn(&'a T): Sized {}
fn g<T>() where { for _ in [0] {} }

And even if not, as you said, after a couple of tokens you know what you have.

Well https://github.com/rust-lang/rfcs/pull/3629#issuecomment-2098534142 explained it quite clearly.

kennytm commented 4 months ago

yeah I know the inner attribute is bad style but will the following apply the attribute to the function item itself?

fn X()
unsafe {
    #![allow(non_snake_case)]
}

and if we want to add an attribute to the block is it still like this?

fn X()
#[allow(unsafe_code)]
unsafe {
}
traviscross commented 4 months ago

cc @rust-lang/style

davidbarsky commented 4 months ago

@davidbarsky wrote:

A downside of this proposal that I don't see mentioned is that it would make error recovery more difficult in IDEs like rust-analyzer or RustRover. Delimiters like parentheses/curly brackets make it trivial for IDEs to assume boundaries between snippets of incomplete code, but this RFC would throw a wrench into such approaches.

This (for both humans and tools) is exactly why I'm proposing to only allow this for constructs that accept a single block. Allowing an arbitrary expression would have the problem you're describing. Allowing e.g. try { ... } or gen { ... } doesn't seem like it would be substantially more complex than the existing { ... }.

What Kenny said in https://github.com/rust-lang/rfcs/pull/3629#issuecomment-2099103233. Malformed/incomplete code—even along those lines—is really common. It's not necessarily more difficult to change a parser, but the code as-it-exists (or rather, as it is written by people) would mean that they'd have a small, but perceptible degradation in their IDE.

If you're to consider a syntactic change like this, I think something like the following:

fn foo(x: i32) -> i32 = gen {
    todo!()
};

...would sidestep most of the incremental parsing issues.

kennytm commented 4 months ago
fn foo(x: i32) -> i32 = gen {
    todo!()
};

this would be #3369

clarfonthey commented 4 months ago

I really don't like this solution for, most of the reasons folks have mentioned. It feels too close to the braceless C statements that Rust tried so hard to avoid, and it just looks weird to me.

I get all the motivation for this, but I don't think that the solution is satisfactory. I would much prefer just indenting my functions one more level and dealing with the consequences of that than adding in new syntax, unless we do find that the equals-expression syntax doesn't have the issues that were brought up in previous discussions.

I'm basically assuming that the indenting is the main issue here, since typing braces is relatively trivial. The language is a balancing act of tradeoffs and I think that this one is extremely minor and not worth introducing entirely new syntax for.

belkadan commented 4 months ago

I personally think it particularly doesn't read very well when the function header wraps onto multiple lines:

fn example(
  x: AVeryLongTypeNameThatPushesTheSignatureOntoMultipleLines,
  y: u32
) -> u32
match x {
  _ => y
}
tmccombs commented 4 months ago

Another prior art is scala , where the = is actually required, and the RHS can be any expression (in fact a body with braces is just a specific case of that, since such a body is itself an expression).

I don't have a strong opinion on it if there is a delimeter such as =, but without it, I think the body feels disjointed from the signature, and it isn't visually apparent the two are part of the same item.

SOF3 commented 4 months ago

could I argue that this is ultimately a rustfmt issue, that it should prefer styles that squeeze short block wrappers on the same line or indent level?

in other words: why do we need to introduce a separate syntax rather than simply changing the way code gets formatted? saving a pair of curly braces has no practical benefit other than instructing the formatter not to increase the indent level.

N4tus commented 4 months ago

Putting the body block as well as the closing '}' on the same line avoids the indentation, while only beeing a style change:

fn example(
  x: AVeryLongTypeNameThatPushesTheSignatureOntoMultipleLines,
  y: u32
) -> impl Iterator<Item = u32> { gen {
  yield x.0;
  yield y;
}}

You could even put the gen block declaration on the next line:

fn example(
  x: AVeryLongTypeNameThatPushesTheSignatureOntoMultipleLines,
  y: u32
) -> impl Iterator<Item = u32> { 
gen {
  yield x.0;
  yield y;
}}

Also in this motivating example:

fn foo() -> NamedFutureType
async {
    ...
}

how would you convert the async block to the user-specified NamedFutureType?

kennytm commented 4 months ago

Also in this motivating example:

fn foo() -> NamedFutureType
async {
    ...
}

how would you convert the async block to the user-specified NamedFutureType?

that's how TAIT works...

#![feature(type_alias_impl_trait)]

use std::future::Future;

type NamedFutureType = impl Future<Output = ()>;

fn foo() -> NamedFutureType {
    async {
    }
}
slanterns commented 4 months ago

I think at least the alternative with a = looks much more better than the current proposal: https://theincredibleholk.org/blog/2023/12/15/rethinking-rusts-function-declaration-syntax/.

fbenkstein commented 4 months ago

How does if / if let without else work? Is the function only allowed to return unit ()?

clarfonthey commented 4 months ago

How does if / if let without else work? Is the function only allowed to return unit ()?

The combined ifs and elses are all considered a single block, so, those are fine. All of these functions can return a type, as mentioned in the examples.

fbenkstein commented 4 months ago

This is from the RFC text:

The full list of block constructs permitted at the top level of a function: ...

  • if and if let, if and only if there's no else.

The combined ifs and elses are all considered a single block, so, those are fine.

I don't understand how these two statements fit together. Maybe it should say "if and only if there is an else"?

Yokinman commented 4 months ago

Even if it's possible to have an expression follow the signature unambiguously I would still prefer some kind of separator. It seems more future-proof, and it would definitely make it easier to read simpler single-line functions.

Human Visual Parsing

If formatted poorly, the block construct could "disappear" into the function signature. We recommend that the default Rust style use a newline to separate the type from the block, making this visually straightforward to parse.

This seems to be the only problem with using a separator - Rust style prefers line breaks before operators and with extra indentation, which would render the purpose of this moot.

fn countup(limit: usize) -> impl Iterator<Item = usize>
    = gen {
        for i in 0..limit {
            yield i;
        }
    };

// vs
fn countup(limit: usize) -> impl Iterator<Item = usize> {
    gen {
        for i in 0..limit {
            yield i;
        }
    }
}

I think we can all agree that eliding a layer of indentation inside a block is never gonna be an accepted style change, since it breaks the intuitive reading that the beginning and ending of any scope will have corresponding indentation. Now you'd have to match brackets yourself, or maybe your IDE will highlight them for you:

fn countup(limit: usize) -> impl Iterator<Item = usize> {
gen {
    for i in 0..limit {
        yield i;
    }
}}

Maybe it could be styled like match arms, where the line break occurs after the arrow?

fn countup(limit: usize) -> impl Iterator<Item = usize> =>
gen {
    for i in 0..limit {
        yield i;
    }
}

On the other hand, obviously putting the keyword before fn solves this by definition, but is it really important for the keyword to be immediately visible if the return type is now explicit?

petrochenkov commented 4 months ago

I would prefer the

fn f() = EXPR;

alternative, if we are doing this at all.

It can work for both block-like bodies

fn f() = match {
    // arms
};

and for very short bodies

fn f(x: u8) = x + 1;

I'm pretty sure there was an old closed RFC about this (by Centril?), but I cannot find it now.

joshtriplett commented 4 months ago

@davidbarsky made a compelling case that parsing would be substantially easier with the = separator. Given that, I'll switch the RFC over to propose that instead.

compiler-errors commented 4 months ago

@joshtriplett: Given the amount of feedback this RFC this has received, I would prefer if that were opened as a separate RFC PR. It's a totally separate proposal, so it would be nice to give people a separate, clean slate to react to the new proposal.

kennytm commented 4 months ago

please address the arguments from #3369 in that new RFC

SOF3 commented 4 months ago

would also be worth considering if anyone prefers a rustfmt-based approach, which involves no compiler change and only has the disadvantage of double closing brace (which is not worse than nested blocks in function calls ({ \n...\n })). This is to add a nightly rustfmt option that allows collapsing the outermost nesting block into the first line if it is short and/or without where bounds:

fn short_fn() { async {
    // ...
}}

fn a_long_line(of: Arguments) -> And<Return, Types> {
    async {
        // ...
    }
}