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

Bind try keyword as higher precedence #8067

Closed
fengb opened this issue Feb 25, 2021 · 12 comments
Closed

Bind try keyword as higher precedence #8067

fengb opened this issue Feb 25, 2021 · 12 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@fengb
Copy link
Contributor

fengb commented Feb 25, 2021

There are a few cases where Zig doesn't match my intuition because try is too low of precendence:

if (try foo == bar) {}
if ((try foo) == bar) {} // I meant this
if (try (foo == bar)) {} // Zig uses this

const baz = try foo orelse return bar;
const baz = (try foo) orelse return bar; // I meant this
const baz = try (foo orelse return bar); // Zig uses this

I'm not quite clear on the rules as precedence table does not include try.

@Vexu
Copy link
Member

Vexu commented Feb 25, 2021

This has been previously rejected in #5436 and #2138.

@raulgrell
Copy link
Contributor

For quick reference, Vexu is referring to the decision:

andrewrk:
We have an accepted proposal for this already, which is to require parentheses to disambiguate: #114

@Vexu Vexu added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Mar 4, 2021
@Vexu Vexu added this to the 0.8.0 milestone Mar 4, 2021
@batiati
Copy link

batiati commented Mar 13, 2021

The main issue I see on status-quo try is the fact it has the opposite precedence of catch, which seems non-intuitive.

// tries the firstFunc
try firstFunc().secondFunc();

// catches the secondFunc
firstFunc().secondFunc() catch |err| {...}

I like try because it is very explicit and visible when reading code, but I would prefer to have a unwrap operator, acting just like try, but without margin for precedence ambiguity. I suppose the .! operator:

firstFunc().!.secondFunc().!;

//equivalent of
try (try firstFunc()).secondFunc();

It would be particularly useful when writing fluent APIs,

var ret = firstFunc().!.secondFunc().!andThen().!;

The possibility of catching together groups of errors would be nice too:

//something like that
var ret = (firstFunc().!.secondFunc().!.andThen().!) catch |err| {...};

//status-quo zig
var ret1 = firstFunc() catch |e1| { ... }
var ret2 = ret1.secondFunc() catch |e2| { ... }
var ret3 = ret2.andThen() catch |e3| { ... }

//this only catches the last function call
var ret = (
(try (try firstFunc()).secondFunc()).andThen()
) catch |err| { ... }

Cheers!

@raulgrell
Copy link
Contributor

Also for reference, the .! used to be part of the language as the error unwrap operator just like the current .? operator for nulls. It was removed in favor of the more verbose catch unreachable as a way to disincentivize its use since although it's good to make ignoring errors ergonomic, it would be overused if it's trivial.

Making it act like try, returning the error for the current function, would go against having all control flow handled by keywords which is one of Zig's design principles.

The grouping issue makes it hard to know which function resulted in an error, which is one of the problems with exception handling in many other languages. When refactoring to allow handling each of the functions properly, setting appropriate defers and errdefers between calls, after all the shuffling around of catch blocks and intermediate variables you'd end up with something like status quo Zig anyway.

@batiati
Copy link

batiati commented Mar 15, 2021

@raulgrell considering maybe_null.? is similar to maybeError() catch unreachable, I totally agree, it must be more verbose and not too easy to ignore errors.

I suggested maybeError().! can behave just like try maybeError() does, bubbling up the error, and therefore preserving Zig's current design, with the benefit of the obvious precedence of the .! operator (just like it is in Rust's .? operator) EDIT: yeah, you said "having all control flow handled by keywords", you're right.

For grouping errors, I imagine it could behave exactly the way it is for fn, but in a block scope:

fn maybeError() !void {
    try canFault1();
    try canFault2();
}

fn caller() void {
    // I don't care if it was from canFault1 or canFault2
    maybeError() catch |err| { .... }
}
// catching errors together in the same way, but without a function
fn proposal() void {
    {
       try canFault1();
       try canFault2();
    } catch |err| { ... }
}

@yohannd1
Copy link

yohannd1 commented Mar 15, 2021

I have a suggestion to introduce a new keyword operator, andthen, which could be used as <expr1> andthen [capture] <expr2>.

// Let's suppose we have `funcN` as a function with signature `fn(i32) error{N}!i32`, and it returns `error.N`.
// Let that be true for funcA-funcZ.
fn funcN() error{N}!i32 {
    return error.N;
}

const result: error{A, B}!i32 = funcA() andthen |x| funcB(x);
std.testing.expect(result == error.A);

// for this one to work `andthen` should have a higher or the same precedence as `catch` (not sure).
const result: error{A, B, C}!i32 = funcA() andthen |x| funcB(x) andthen |x| funcC(x);
std.testing.expect(result == error.A);

With this addition, the code @batiati previously suggested could be rewritten as this:

// @batiati's old suggestion:
var ret = (firstFunc().!.secondFunc().!.andThen().!) catch |err| {...};

// my suggestion:
var ret = firstFunc() andthen |x| x.secondFunc() andthen |x| x.andThen() catch |err| { ... };

Not only that, but we can even extend the usage of andthen so it can be also used in optional types, although I don't know if they would be very useful here:

const value: ?i32 = null;
const result: ?i64 = value andthen |x| fallibleNumberOperation(x) andthen |x| x * 2;
std.testing.expect(result == null);

There's also #5610, though I think this suggestion of mine is better when working with function/method chaining:

// Attempt one with block-local error handling
blk: {
    // Since we can't shadow variable declarations, we'll have to create a variable for each attempt.
    const v = try :blk getValue();
    const v2 = try :blk v.doSomethingWithValue();
    break :blk v2.doYetAnotherThingWithValue(); // you could also accidentally pass `v` here if the method name is the same
}

// Attempt two with block-local error handling
blk: {
    break :blk (try :blk (try :blk getValue()).doSomethingWithValue()).doYetAnotherThingWithValue();
}

// Using andthen to clean things up
// The two lines below are equivalent. Because of that, you can use the same |v| capture without causing shadowing issues.
getValue() andthen |v| v.doSomethingWithValue() andthen |v| v.doYetAnotherThingWithValue();
(getValue() andthen |v| v.doSomethingWithValue()) andthen |v| v.doYetAnotherThingWithValue();

@matu3ba
Copy link
Contributor

matu3ba commented Mar 19, 2021

I'm not quite clear on the rules as precedence table does not include try.

If the spec is not wrong, look here: https://github.com/ziglang/zig-spec/blob/4e4c1a9c27707996e39206830c842639e916307d/grammar/grammar.y#L283

cc @SpexGuy Is there some enforced order of the prefix keywords? The prefix keywords dont look like there must be an order ie that the first word must be a try (and subsequent words are not allowed to be a try).

@batiati
Copy link

batiati commented Mar 20, 2021

I still thinking about it, and I realized that maybe it is not a precedence matter ...

Considering:

pub const Something = struct {

	const Self = @This();
	
	fn maybeError(self: *Self) anyerror!*Self {
		//...
		return self;
	}

	fn noError(self: *Self) *Self {
		//...
		return self;
	}
}

Since we cannot do that:

var something = Something {};

//compiler error here, makes no sense to try "noError"
_ = try something.noError();

And we cannot do that either:

var something = Something {};

//compiler error here, no function "noError" found in an ErrorUnion
_ = something.maybeError().noError();

We naturally expect the compiler to try the only possible ErrorUnion:

var something = Something {};

//just one possible error, should be OK
_ = try something.maybeError().noError();

//just one possible error, should be OK too
_ = try something.noError().maybeError();


//compiler error, no function "maybeError" found in an ErrorUnion
_ = try something.maybeError().maybeError();

Extending this to the problem of fluent APIs, there is a way to introduce a syntax-sugar operator, still having all control flow handled by keywords.
Introducing the !. operator not as try replacement, but as an "error-propagation" operator, could it work like that:

var something = Something {};

//status quo, OK
_ = try something.maybeError();
_ = something.maybeError() catch |err| { ... }

//The error propagation !. operator should be allowed only in "try" or "catch" expressions

//Try the first and second calls
_ = try something.maybeError()!.maybeError();

//Catches the first and second calls
_ = something.maybeError()!.maybeError() catch |err| { ... };

//Returns an error union
var maybe = something.maybeError()!.maybeError();

if (something.maybeError()!.maybeError()) |payload| {
    //...
} else |err| {
   //...
}

//Wrong usage
_ = something.maybeError()!.; //Wrong
_ = try something.maybeError()!.; //Wrong
_ = something.maybeError()!. catch |err| { ... }; //Wrong

@matu3ba
Copy link
Contributor

matu3ba commented Mar 20, 2021

Introducing the !. operator not as try replacement, but as an "error-propagation" operator, could it work like that:

That looks very easy to miss, because !. is hard to read.

Piping commands together is also counter-intuitive for understand the control-flow of the program.
For shells one uses at least command1 | command2, if one uses best practices.

@yohannd1
Copy link

yohannd1 commented Apr 9, 2021

@matu3ba:

Piping commands together is also counter-intuitive for understand the control-flow of the program.
For shells one uses at least command1 | command2, if one uses best practices.

Could you elaborate on this please? I don't understand what would be counter-intuitive about it.

Sorry for taking so much time to respond, I didn't understand if you mentioned my suggestion ^^'

@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@alexrp
Copy link
Member

alexrp commented Aug 18, 2021

For what it's worth, I come from the C# world where prefix keyword operators work as in this proposal, i.e. this is valid.

So I expected to be able to write code like this:

        if (try reader.readInt(u16) != dos_signature)
            return error.InvalidDosSignature;

Only to then discover that I needed to put parentheses around the left-hand side of the != operator. I think that's a little unfortunate. My background might be coloring my perspective here, but I have a hard time imagining anyone finding this bit of code (without parentheses) ambiguous.

@andrewrk
Copy link
Member

#8067 (comment)

☝️

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

8 participants