ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.41k stars 2.43k forks source link

Remove loop-else expressions #1004

Closed tiehuis closed 6 years ago

tiehuis commented 6 years ago

We currently allow loops (for/while) to have an else expression on them. The code within this expression is evaluated only if the loop runs in its entirety.

for (values) |value| {
    if (value == s) {
        // do something
        break;
    }
} else {
     // do the fallback thing
}

I argue that this type of loop and flow is uncommon enough that we should remove this construct completely. The else on a loop could be fairly non-obvious if not familiar with the construct and it adds another thing to the language which is non-essential.

Given that we only should have "Only one obvious way to do things", here are some other ways of replicating the above control flow:

A simple flag

var found =  false;
for (values) |value| {
    if (value == s) {
        found = true;
        break;
    }
}
if (found) {
// do the fallback thing
} else {
    // do the fallback thing
}

Write a function

fn findItem(comptime T: type, values: []const T, item: T) bool {
    for (values) |value| {
        if (value == item) {
            return true;
        }
    }

    return false;
}

if (findItem(...)) {
    // do the fallback thing
} else {
    // do the fallback thing
}

Use a named block in place of a function

const found = block: {
    for (values) |value| {
        if (value == item) {
            break :block false;
        }
    }    
    break :block true;
}

if (found) {
    // do the fallback thing
} else {
    // do the fallback thing
}

Are there any other specific use-cases I'm missing? How many times has this been done in the stdlib? I know I've never used this construct.

0joshuaolson1 commented 6 years ago

Do most Zig constructs behave as expressions? Keeping it here would be nice, but I don't see that in the "other ways" list.

andrewrk commented 6 years ago

I think there is a strong argument in favor of this proposal - a smaller language with less features.

The only reason I added this feature is that you could view it as actually making the language smaller and simpler. For example, you can think of if and while as being the same construct, with the only difference that at the end of the body, control flow goes back to the condition rather than the end.

One example of how all these constructs are equivalent is that they all work with nullable types and error types. For example:

while (canFail()) |success_value| {
    // do something with success_value
} else |err| {
    // do something with err
}

it looks very similar to if:

if (canFail()) |success_value| {
    // do something with success_value
} else |err| {
    // do something with err
}

so I suppose the question is, which way is simpler? having the if, while, and for constructs be closer in behavior? or removing an uncommonly used ability of loops?

For the sake of completeness, here's the while code above without else:

while (true) {
    if (canFail()) |success_value| {
        // do something with success_value
    } else |err| {
        // do something with err
        break;
    }
}

One thing to note is that if we don't have the ability for while to have else |err|, then it cannot support an error union condition - because syntactically while (a) |b| {} requires a to be nullable, and it's the else |_| that makes it an error union.

And then if we removed ability for while conditions to be error unions, would we also remove the ability for them to be nullable? That's certainly a feature we use a lot.

BraedonWooding commented 6 years ago

Perhaps maybe the following would be preferable? That is to have for/while return an enum.

var ok = for (values) |value| {
    if (value == s) {
        // do something
        break;
    }
}

if (ok == loopExit.Okay) {
    // No breaks, just normal
} else if (ok == loopExit.Break) {
   // A break occurred
} else if (ok == loopExit.Nullable) {
   // A nullable failed
}

Note I don't know what to call the things so I just wanted to chuck them a basic name to atleast outline the idea.

This would obviously decompose down to just the flag, but this way atleast it looks a little cleaner and you don't forget the flag set which could be a common cause of a bug! This way however you lead to longer code as you have to check the condition, though I guess this does offer the ability to check for things that you couldn't previously such as break vs nullable vs success. Or maybe we offer builtins to cover it.

bb010g commented 6 years ago

I don't like that idea due to how it conflicts with doing labeled returns out of labeled loops. :+1: for the current impl from me.

PavelVozenilek commented 6 years ago

This feature is small, local, and intuitive to me.

The alternatives would require inventing local flag name and risk misuse of the flag later (unless #474 gets adopted). I see named blocks as abomination.

Furthermore, it scales up. Imagine three nested expressions. Still readable, no useless and tricky stuff. Flag name would appear on visually remote lines.

raulgrell commented 6 years ago

I think the else branch is a valuable feature that doesn't really make the language more complex. Like @andrewrk said, it is consistent with the rest of the language and fits well with nullable types and error handling which are arguably among Zig's core features.

It is not an entirely new concept: Python has else branches after loops that behave the same way. While it may be unfamiliar to new users, I'd argue that it's intuitive enough for the first explanation to stick.

EDIT: I also believe that any construct that will help achieve a goal without being forcing the user to create an variable/identifier is inherently valuable.

skyfex commented 6 years ago

Given that we only should have "Only one obvious way to do things"

I think this principle has a tendency to be over-emphasised a bit in Zig. Given that you can do inline assembly, there is always two ways to do something.

To me, elegancy, clarity and conceptual simplicity comes before this principle.

Put another way, you could implement if (..) { .. } else { .. } with flags as well. So should we drop else { .. } entirely?

There's also a way to say that using flags and function is not "doing the same thing". else and break can probably be optimized better in some cases. This could be important to implement fast/efficient code in critical sections. Given that Zig drops support for goto, it could be important to have features that cover the most common uses of goto. Fast code is a core use case of Zig after all.

tiehuis commented 6 years ago

I'm happy enough with the arguments against this so am going to close this issue. Feel free to continue to make any arguments contrary to this if you don't agree, however.