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

Language feature: Named variables in function return declarations #1999

Closed
IvankoB opened this issue May 11, 2017 · 41 comments
Closed

Language feature: Named variables in function return declarations #1999

IvankoB opened this issue May 11, 2017 · 41 comments
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.

Comments

@IvankoB
Copy link

IvankoB commented May 11, 2017

With visibility of function parameters, possibly with default return values, to achive functionality of Delphi's function "Result" auto variable or as shown in the below "Go" code:

func OpenPrinter(name string) (success bool, printer HANDLE) {
openPrinter := winspool.NewProc("OpenPrinterA")
Cname := unsafe.Pointer(C.CString(strToAnsi(name)))
defer C.free(Cname)
Cret, _, _ := openPrinter.Call(uintptr(Cname), uintptr(unsafe.Pointer(&printer)), uintptr(0 /Cnil/))
success = (int(Cret) != 0)
return
}

Note the usage of return varibales "success" & "printer" within the function body. Very handy & clear.
The current "Rust" code for this function is as follows :

fn open_printer(name: String) -> (bool, HANDLE) {
let h: HANDLE = ptr::null_mut();
unsafe {
(OpenPrinterW(str2wcstr(&*name).as_ptr(), &h, std::ptr::null_mut()) == TRUE, h)
}
}
@mark-i-m
Copy link
Member

I don't have strong feelings, but it feels "unrustic" to return a value by assigning to named return variable. Also, this might just be me, but I find the rust code you posted much more readable than the Go code you posted...

@SimonSapin
Copy link
Contributor

This sounds like "structural records". Rust used to have them, and removed them in 0.6:

Adding them back was proposed in https://internals.rust-lang.org/t/pre-rfc-unnamed-struct-types/3872 . As far as I can tell this proposal was not explicitly rejected, the conversation just died out. It may be worth writing a formal RFC pull request.

@burdges
Copy link

burdges commented May 11, 2017

You could rephrase this as asking to place a pattern in return position and assign to it. And you might as well ask for patterns in argument position while you're at it. I donno if either really buys much, but it might look like

fn foo((a,mut b) : (usize,usize)) -> Range { mut start, mut end } : Range<usize> { ... }

or maybe you could omit the muts in return position so long as you only assigned to each once.

@IvankoB
Copy link
Author

IvankoB commented May 11, 2017

One more advantage of this approach is that no need explicitly to maintain stack variables to return within function - those (if declared as proposed) are automatically created & managed on function calling & returning. Who programmed in Delphi definetely loved its "result" auto variable.

@IvankoB
Copy link
Author

IvankoB commented May 11, 2017

You could rephrase this as asking to place a pattern in return position and assign to it

Yes. In order that not to create this pattern within function.

@IvankoB
Copy link
Author

IvankoB commented May 11, 2017

code you posted much more readable than the Go code

Yes, it's short but note the "&h -> h" l-value usage in the return calculation.

@burdges
Copy link

burdges commented May 11, 2017

I think Rust already requires the caller allocate the space for returned structs @IvankoB but maybe one could optimize it for tuples by passing pointers for each component instead of one pointer for the whole tuple.

I think non-lexical lifetimes might enable this without adding complexity to the fn declaration, so maybe

fn foo(..) -> Range<Foo> {
    let Range { start, mut end } = r;
    end = 0;
    for ... { end += baz; }
    start = bar;
    r
}

as opposed to

fn foo(..) -> Range<Foo> {
    let mut r = 0..0;
    {
        let Range { start, mut end } = r;
        end = 0;
        for ... { end += baz; }
        start = bar;
    }
    r
}

@IvankoB
Copy link
Author

IvankoB commented May 11, 2017

let Range { start, mut end } = r;

It's an explicit creation of stack variables which will be returned when function finishes. The ability of returning stack variables as ref counted function results is very handy fetarure of Rust. With the proposal in simple cases it can be simplified even more.

@wesleywiser
Copy link
Member

Wouldn't the "rustic" way of writing this actually use Result or at least Option? For example, this is what the libstd does:

    pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
        let path = to_u16s(path)?;
        let handle = unsafe {
            c::CreateFileW(path.as_ptr(),
                           opts.get_access_mode()?,
                           opts.share_mode,
                           opts.security_attributes as *mut _,
                           opts.get_creation_mode()?,
                           opts.get_flags_and_attributes(),
                           ptr::null_mut())
        };
        if handle == c::INVALID_HANDLE_VALUE {
            Err(Error::last_os_error())
        } else {
            Ok(File { handle: Handle::new(handle) })
        }
    }

https://github.com/rust-lang/rust/blob/2cc3358e4f1c4a79685745a461a1be1ce784b88a/src/libstd/sys/windows/fs.rs#L257

@IvankoB
Copy link
Author

IvankoB commented May 11, 2017

Our collegue "burdges" has got the idea.

@mark-i-m
Copy link
Member

fn foo(..) -> Range<Foo> {
    let Range { start, mut end } = r;
    end = 0;
    for ... { end += baz; }
    start = bar;
    r
}

Sorry, I'm still not convinced. For me this piece of code was genuinely confusing for a minute until I realized r is the implicit return value. I can't think of anywhere else in rust where variables can just appear out of nowhere like that...

@withoutboats withoutboats added the T-lang Relevant to the language team, which will review and decide on the RFC. label May 12, 2017
@burdges
Copy link

burdges commented May 12, 2017

Yeah I realized it'd need an explicit name like fn @mark-i-m

I've no idea if this is a good idea, but I wanted to point out that destructuring can happen in the body without fancy new syntax in the fn declaration.

@IvankoB
Copy link
Author

IvankoB commented May 12, 2017

This feature also involves empty return ("Go") or no return at all ("Delphi").

@mark-i-m
Copy link
Member

@IvankoB what do you mean by "empty return" and "no return at all"? Are they analogous to return () or ! ?

@burdges
Copy link

burdges commented May 12, 2017

I'm nervous about the underlying goal here because we should avoid mutability when possible. Also, can return values from the fn line be replaced with new bindings? All sounds messy.

I think the question should be : After non-lexical lifetimes work, should Rust support initial assignments to destructured uninitalized data? If so, this might look like

fn foo(..) -> Range<Foo> {
    let r @ Range { start, mut end };
    // set and mutate end and set start
    r
}

which works even when r is not the return value. Also, this form works with enums and early returns, so you can write

fn foo(..) -> Result<Foo,MyError> {
    let r @ Ok(mut f);
    ...
    f = bar() ?;
    ... 
    r
}

@mark-i-m
Copy link
Member

@burdges That's true but is it more readable than? That's unclear to me at the moment...

fn foo(..) -> Range<Foo> {
    let start = ...;
    let mut end = ...;
    // set and mutate end and set start
    Range{ start, end }
}

@IvankoB
Copy link
Author

IvankoB commented May 12, 2017

Are they analogous to return () or ! ?

In "Go" it indicates to exit function immediately with return value set by last change within the function body if it took place otherewise the value on calling the function:

func OpenPrinter(name string) (success bool, printer HANDLE) {
openPrinter := winspool.NewProc("OpenPrinterA")
Cname := unsafe.Pointer(C.CString(strToAnsi(name)))
defer C.free(Cname)
Cret, _, _ := openPrinter.Call(uintptr(Cname), uintptr(unsafe.Pointer(&printer)), uintptr(0 /Cnil/))
success = (int(Cret) != 0)
return  ///// <<<<<<<<<<< HERE
}

@IvankoB
Copy link
Author

IvankoB commented May 12, 2017

we should avoid mutability when possible.

Since they're decladed in same scope as input arguments all their lifetimes can be managed alltogether.

@mark-i-m
Copy link
Member

@IvankoB You can do an explicit return in rust too.

fn foo() {
    if (blah == blah) {
         println!("Hey, you!");
         return;
    }
    if(bar) {
        baz();
    } else {
         fudge();
     }
}

For example, this is what the ? operator does implicitly...

fn read_stuff(my_path: &str) -> Result<...> {
    let mut f = File::open(my_path)?; // may return early
    let mut str = String::new();
    f.read_to_string(&mut str)?; // may return early
    Ok(str)
}

@IvankoB
Copy link
Author

IvankoB commented May 12, 2017

You can do an explicit return in rust too.

Ones declaring some return (= not procedures) as well ?

@mark-i-m
Copy link
Member

mark-i-m commented May 12, 2017 via email

@burdges
Copy link

burdges commented May 12, 2017

Yes, I think field puns largely eliminate the need for this @mark-i-m Why should it matter if you make your pun before or after the meal? It's just interesting to see what non-lexical lifetimes enable. :)

As an aside, I think structural records might be interesting for partially initializing structs, btw. If we have pub struct Foo { a: A, b: B, c: C, d: D } then Foo::{b,c} could be the structural record { b: B, c: C }, so you could write

impl Foo {
    fn new_partial(..) -> Foo::{b,c} { .. }
    pub fn new(..) -> Foo {
        ...
        Foo { a, d, ...newpartial(..) }
    }
}

Anyways, I think one solid argument against Go style named returns, as proposed here, is simply that it's too much strangeness budget on too small and too questionable a feature.

@IvankoB
Copy link
Author

IvankoB commented May 12, 2017

The example code might be smth like:

fn open_printer(name: String) -> (result: bool, h: HANDLE) {
unsafe {
result = (OpenPrinterW(str2wcstr(&*name).as_ptr(), &(h =ptr::null_mut()) , std::ptr::null_mut()) == TRUE);
}
}

@IvankoB
Copy link
Author

IvankoB commented May 13, 2017

BTW, how do peole think abou allwing the syntax sugar:

&(h =ptr::null_mut())

for :

let h: HANDLE = ptr::null_mut(); <<< HERE
unsafe {
(OpenPrinterW(str2wcstr(&*name).as_ptr(), 
&h <<<< HERE

@Ixrec
Copy link
Contributor

Ixrec commented May 13, 2017

That example seems like it's missing a few pieces so I can't tell for sure what you're suggesting, but my guess is that you're trying to say we should allow &(h =ptr::null_mut()) at the top of open_printer() to do the same thing as Err(ptr::null_mut()) would at the end. If so, my thought is that seems really confusing and I don't see any benefits to it.

@IvankoB
Copy link
Author

IvankoB commented May 13, 2017

we should allow &(h =ptr::null_mut())

The syntax sugar as combining "declaring + initalizing + borrowing" instead of separate "(declaring + initalizing)" and "borrowing" ? Why not BTW if Rust checks context & lifetimes thoroughly before compiling ?
PS: anyway me don't insist on, it's just a proposal(s).

@rayascott-zz
Copy link

rayascott-zz commented Nov 7, 2017

It's day 2 of learning Rust for me, and I must say that without naming the return value, how do you know what the value represents, or means? I find it baffling that you'd expect to name function parameters, but not return values. I'm not just coming from Go, but also Swift, and both languages allow the return of tuples. In Objective-C you can't return a tuple, but you can at least name the return value in your method, such as

+ (NSURL *)fileURLWithPath:(NSString *)path isDirectory:(BOOL)isDir;

The idea of having to guess what the return value represents doesn't instil much hope for writing semantically clear code.

Looking at this Swift example, it is abundantly clear as to what the function accepts and returns. It's just as much, if not more so about the perspective from outside the function, than in. Put simply, it's a much clearer interface.

func range(array: [Int]) -> (min: Int, max: Int) {
    var currentMin = array[0]
    var currentMax = array[0]
    for value in array[1..<array.count] {
        if value < currentMin {
            currentMin = value
        } else if value > currentMax {
            currentMax = value
        }
    }
    return (currentMin, currentMax)
}

let bounds = range(array: [8, -6, 2, 109, 3, 71])
print("min is \(bounds.min) and max is \(bounds.max)")

@mark-i-m
Copy link
Member

mark-i-m commented Nov 7, 2017

IMHO, the name of the function/method should communicate what it returns/does in a way that makes named return values redundant. Indeed, isDirectory seems like an unfortunately example in this aspect.

Also, rust has a strong commenting culture centering around doc comments...

@rayascott-zz
Copy link

isDirectory isn't a 2nd example, that's all one method definition. I suppose if the code is obscure, then you would tend to lean heavily on comments. Considering autocomplete would negate that, it still seems rather contradictory that the inputs are named, but not the outputs.

@mark-i-m
Copy link
Member

mark-i-m commented Nov 7, 2017

isDirectory isn't a 2nd example, that's all one method definition

Ah, thanks for clarifying that :)

Considering autocomplete would negate that, it still seems rather contradictory that the inputs are named, but not the outputs.

They are named: the name of the method/function describes the output/expected behavior.

@rayascott-zz
Copy link

Are you one of the core developers of Rust?

@wesleywiser
Copy link
Member

Why not just do:

struct MinMax = { min: u64, max: u64 }

fn range(s: &[u64]) -> MinMax {
 ....
}

@rayascott-zz
Copy link

Rather negates the whole point of tuples as return values though I would think.

@sfackler
Copy link
Member

sfackler commented Nov 7, 2017

Is there some inherent need to return tuples from functions?

@rayascott-zz
Copy link

Fairly unusual question. Is there a need? Only in so much as there is an inherent need to pass multiple parameters to a function. Just because you can’t find a use for a feature, doesn’t mean others shouldn’t too, or it should be removed.

@sfackler
Copy link
Member

sfackler commented Nov 7, 2017

What feature is being removed here? I'm confused.

@burdges
Copy link

burdges commented Nov 7, 2017

I think named variables ala Go sound like a huge mistake because they discourage using ADTs correctly, and encourage excessive mutability as noted previously. In other words, if you want to return (A,B,C) right now, then there are good odds you'll figure out you actually need to return something like (A,Result<(B,Option<C>),Err>) tomorrow, or a custom enum, and named variables make this messy. Actually, they make it messy even in Go where you simply want to make nil appear in the right slots.

I think @rayascott's Swift example might be covered by the structural records suggestion #1999 (comment) which might look like this with named field puns:

fn range(s: &[u64]) -> Option<struct { pub min: u64, pub max: u64 }> {
    if s.len() == 0 { return None; }
    let mut min = s[0];
    let mut max = s[0];
    for v in s[1..] {
    ...
    }
    Some(struct { min, max })
}

I donno if this improves so much on @wesleywiser's version though. In fact, it's usually wise to consider if an intermediate data type might help in more ways, ala the builder pattern. Also, structural records have much better uses, so I'd hate for any serious interest in them to be sidelined by trying to make them work in every last position, including fn return values.

As an aside, if we write this using iterator methods then we encounter a minmax function one might be tempted to expose as an inherent method of MinMax:

fn rang<I: IntoIterator<Item=u64>>(itr: I) -> Option<(u64,u64)> {
    let mut itr = itr.into_iter();
    let i0 = itr.next();
    let minmax = |x,y| (min(x.0,y),max(x.1,y));
    i0.map(|a| itr.fold((a,a), minmax ))
}

@mark-i-m
Copy link
Member

mark-i-m commented Nov 7, 2017

Are you one of the core developers of Rust?

Lol, no... why?

@burdges
Copy link

burdges commented Nov 7, 2017

Actually the MinMax type exists as std::ops::Range and the minmax function should be

impl<Idx: Ord<Idx>> Range<Idx> {
    pub fn convex_insert(&self, item: Idx) -> Range<Idx> {
        Range {
            start: self.start.min(item)
            end: self.end.max(item)
        }
    }
}

so you could write

impl<Idx: Ord<Idx>+Copy> Range<Idx> {
    pub fn convex_hull<I: IntoIterator<Item=Idx>>(itr: I) -> Option<Range<Idx>> {
        let mut itr = itr.into_iter();
        let i0 = itr.next();
        i0.map(|a| itr.fold(a..a, Range::convex_insert ))
    }
}

@IvankoB
Copy link
Author

IvankoB commented Nov 11, 2017

The idea is also to preallocate result variables so that they can be used in function body directly (without local declaration) - like the Delphi's "Result" variable. A kind of non-staic mutable variable.

@Centril
Copy link
Contributor

Centril commented Nov 2, 2018

I've proposed structural records (#2584), which are not this proposal, but close enough so I think we can close this now.

@Centril Centril closed this as completed Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

10 participants