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

result location: ability to refer to the return result location before the return statement #2765

Open
andrewrk opened this issue Jun 27, 2019 · 11 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

This issue is split from #287.

Now that we have result location semantics, the following code does not introduce an intermediate value with a copy:

const Point = struct {
    x: i32,
    y: i32,
};

fn foo() Point {
    return bar();
}

fn bar() Point {
    return Point{
        .x = 1,
        .y = 2,
    };
}

test "result location" {
    var point = foo();
}

Previously, the code return bar() would introduce an extra copy, so the body of the function foo would needlessly copy the point before returning it. This copying would happen at every expression, recursively when the type is an aggregate type (such as struct). Now that the result location mechanism is merged into master, you can see that the foo function does not introduce an extra copy:

define internal fastcc void @foo(%Point* nonnull sret) unnamed_addr #2 !dbg !35 {
Entry:
  call fastcc void @bar(%Point* sret %0), !dbg !44
  ret void, !dbg !46
}

However, if you capture the result in a variable and then return the variable, there is an intermediate value - the result variable - which is copied at the return statement:

fn foo() Point {
    const result = bar();
    return result;
}

Now there is a copy, because the Result Location of bar() is the result local variable, rather than the return result location:

define internal fastcc void @foo(%Point* nonnull sret) unnamed_addr #2 !dbg !35 {
Entry:
  %result = alloca %Point, align 4
  call fastcc void @bar(%Point* sret %result), !dbg !47
  call void @llvm.dbg.declare(metadata %Point* %result, metadata !45, metadata !DIExpression()), !dbg !48
  %1 = bitcast %Point* %result to i8*, !dbg !49
  %2 = bitcast %Point* %0 to i8*, !dbg !49
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %2, i8* align 4 %1, i64 8, i1 false), !dbg !49
  ret void, !dbg !50
}

This issue is to make it so that there is a way to refer to the result location, and even call methods on it, before returning it, all without introducing an intermediate value.

For the issue about getting rid of intermediate values when optionals and error unions are involved, see #2761.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Jun 27, 2019
@andrewrk andrewrk added this to the 0.6.0 milestone Jun 27, 2019
@andrewrk
Copy link
Member Author

One possible solution, noted in #287 (comment) is:

Zig will detect when all control flow paths end with return foo;, where foo is the same in all the return expressions, and is declared in a way that allows it to reference the return value. In this case the variable declaration will reference the return value rather than be a stack allocation. The detection doesn't have to be very advanced, just good enough that it's easy to get the detection to happen when you are trying to.

@mikdusan

This comment has been minimized.

@shawnl
Copy link
Contributor

shawnl commented Oct 14, 2019

The detection doesn't have to be very advanced, just good enough that it's easy to get the detection to happen when you are trying to.

This is important, because you I don't think you can make this work all the time without phi nodes.

@thejoshwolfe
Copy link
Contributor

discussed with @SpexGuy and @andrewrk .

The concrete proposal is:

  • If every return statement in a function refers to the same named variable, then that variable's address is the result location.
  • The same applies to break statement values and the result location of a block.

This proposal only applies to functions where the type of the named variable is exactly equal to the return type. See also #2761 for when the variable is of type T and the return type is ?T or !T.

@marler8997
Copy link
Contributor

marler8997 commented Dec 27, 2020

Another idea to consider would be to introduce "return value reference semantics". The difference with this would be that we'd be introducing a way to explicitly state the intention of setting the return value before we return from the function rather than relying on the optimizer to elide a temporary copy of the return value. The problem with relying on an optimization is that:

  1. it's difficult to know how to make code work with the optimization
  2. it's difficult to know when the optimization has been applied

For example, the proposed semantics of eliding this second copy of the return value may cover some cases but miss out on others. This would mean that some code that is able to be optimized won't be, but not because the code can't be optimized, rather, it's the result of an inadequate optimization design. This means the code needs to be structured in a way to conform to the optimization design, which likely only supports a subset of possible scenarios that could be optimized. Futhermore, making code fit this optimization design is difficult because it requires dealing with the 2 difficult issues listed above. By making the feature explicit, code will be forced to be written in such a way as to accommodate the desired effect.

Making this intention explicit means that all possible cases will be supported and the developer knows that their intention is being communicated. As for the syntax, we could use builtin functions for this such as @returnRef() and/or @breakRef("mylabel").

fn foo() Point {
    @returnRef().* = bar();
}

...
    const foo = init: {
        @breakRef("init").* = bar();
        break :init;
    };

Note that once a return value has been set, the code can return/break from the function/scope without specifying a value because the value has already been set. The compiler can use normal code flow analysis to be sure that the return value has been set to know when it's ok to omit the return/break value.

Also note that with these builtin return/break value references, the references can be explicitly forwarded to other functions. For example:

pub fn foo() [3000]u8 {
    bar(@returnRef()); // Note that @TypeOf(@returnRef()) is *[3000]u8
}
pub fn bar(s: []u8) void {
    //...
}

P.S. also note that adding explicit support for accessing the return value doesn't mean we couldn't also add the proposed optimization, both mechanisms have their uses and could exist alongside each other

@N00byEdge
Copy link
Contributor

N00byEdge commented Jul 26, 2022

Or a slight variation: Add a return block which is the only place for return location pointers to be accessed

const T = struct {
  alloc: std.mem.allocator,
  buffer: [128]u8,
  
  fn init() T {
    return {
      @returnPtr().allocator = std.mem.fixedBufferAllocator(&@returnPtr().buffer);
    };
  }
};

var result = T.init();

Instead of doing what you have to do today:

var result: T = undefined;
result.init(); // `init` takes a `self` pointer

The return block would contain statements as usual but upon reaching the end of the return block, the function would return.

@N00byEdge
Copy link
Contributor

Alternative syntax with return blocks (but same semantics)

return |*return_location| {
  // ...
};

2022-07-26-20:22:48

@whatisaphone
Copy link
Contributor

What about giving the result location a name like any other variable?

fn makeArray() result: []u8 {
    fillArray(result);
}

fn fillArray(array: []u8) { ... }

Of course, that becomes awkward for the common case of returning a value directly, so return-with-value should be retained as shorthand to keep existing code working as it already does today.

fn add(x: i32, y: i32) result: i32 {
    return x + y;
    // is shorthand for:
    result = x + y;
    return;
}

Also in the common case, where the return keyboard is used, the name is not used and can be omitted. Now we have the current syntax back, just with more flexible building blocks underneath.

fn add(x: i32, y: i32) i32 {
    return x + y;
    // is shorthand for:
    <unnamed_result_location> = x + y;
    return;
}

This would extend nicely to blocks. Like functions, blocks would get their own result variables:

const a = blk: {  
    fillArray(blk);
};

break would become shorthand similar to return:

const b = blk: {
    break :blk 1;
    // is shorthand for:
    blk = 1;
    break :blk;
};

Although if this feature existed it might be more natural to write the above like this:

const b = blk: {
    blk = 1;
};

@N00byEdge
Copy link
Contributor

I definitely think that's interesting, but then with the result identifier as a pointer to the return value location.

@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jul 9, 2023
andrewrk added a commit that referenced this issue Jul 27, 2023
This commit does two things which seem unrelated at first, but,
together, solve a miscompilation, and potentially slightly speed up
compiler perf, at the expense of making #2765 trickier to implement in
the future.

Sema: avoid returning a false positive for whether an inferred error set
is comptime-known to be empty.

AstGen: mark function calls as not being interested in a result
location. This prevents the test case "ret_ptr doesn't cause own
inferred error set to be resolved" from being regressed. If we want to
accept and implement #2765 in the future, it will require solving this
problem a different way, but the principle of YAGNI tells us to go ahead
with this change.

Old ZIR looks like this:

  %97 = ret_ptr()
  %101 = store_node(%97, %100)
  %102 = load(%97)
  %103 = ret_is_non_err(%102)

New ZIR looks like this:

  %97 = ret_type()
  %101 = as_node(%97, %100)
  %102 = ret_is_non_err(%101)

closes #15669
andrewrk added a commit that referenced this issue Jul 27, 2023
This commit does two things which seem unrelated at first, but,
together, solve a miscompilation, and potentially slightly speed up
compiler perf, at the expense of making #2765 trickier to implement in
the future.

Sema: avoid returning a false positive for whether an inferred error set
is comptime-known to be empty.

AstGen: mark function calls as not being interested in a result
location. This prevents the test case "ret_ptr doesn't cause own
inferred error set to be resolved" from being regressed. If we want to
accept and implement #2765 in the future, it will require solving this
problem a different way, but the principle of YAGNI tells us to go ahead
with this change.

Old ZIR looks like this:

  %97 = ret_ptr()
  %101 = store_node(%97, %100)
  %102 = load(%97)
  %103 = ret_is_non_err(%102)

New ZIR looks like this:

  %97 = ret_type()
  %101 = as_node(%97, %100)
  %102 = ret_is_non_err(%101)

closes #15669
@matklad
Copy link
Contributor

matklad commented Sep 6, 2023

This is mostly tangential curiosity, but having an explicit syntax for result of the functions gives us require/ensure from design-by-contract for free:

fn sqrt(x: i32): i32 {
          assert(x >= 0);                    // precondition
    defer assert(@result() * @result() <= x) // postcondition
}

@MichaelBelousov
Copy link

Perhaps this would provide the necessary piece to implement an ergonomic Result generic (error payloads) that is not intrinsic to the language. (See #2647)

Today, hand rolled Result is unergonomic because it doesn't work with errdefer so requires a lot of boilerplate to handle properly. If you can reference the return address though, you can remove a lot of the boilerplate though.

fn Result(comptime T: type, comptime E: type) type {
  return union (enum) { ok: T, err: E };
}

fn future_maybe(alloc: std.mem.Allocator) Result(i32, []const u8) {
  var mylist = std.ArrayList(i32).init(alloc);
  mylist.append(2);
  // this is basically implementing errdefer yourself on top of a custom union
  defer if (@return() == .err) mylist.deinit();
  // return your error pretty much directly
  erroringFunction() catch return .{ .err = "oh no" };
  return .{.ok = 10};
}

// here is what I do today:
fn today(alloc: std.mem.Allocator) Result(i32, []const u8) {
  var result: Result(i32, []const u8) = .{.err = "no valid result yet"};
 
  var mylist = std.ArrayList(i32).init(alloc);
  mylist.append(2);
  defer if (result == .err) mylist.deinit();
  
  erroringFunction() catch {
    result = .{ .err = "oh no" };
    return result;
  }
  
  result = .{.ok = 10};
  return result;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

9 participants