google / skylark

Skylark in Go: the Skylark configuration language, implemented in Go [MOVED to go.starlark.net]
BSD 3-Clause "New" or "Revised" License
1.19k stars 74 forks source link

Add for/while else #143

Open pauldraper opened 4 years ago

pauldraper commented 4 years ago

I tried to use for-else, only to discover it does not exist (syntax error at 'else': expected expression).

for root in roots:
    if src.path.startswith(root):
        modules.append(create_module(name = src.path[len(root):], file = src))
        break
else:
    fail("Module roots does not contain %s" % src.path, "srcs")

This is surprising to someone familiar with Python control structures.

alandonovan commented 4 years ago

Yes, Starlark does not support for/else, and this may be surprising to a Python programmer. On the other hand, a programmer familiar with every language but Python might stop and wonder what for/else means. Between that, and the complexity of adding "more stuff", I don't find it a compelling feature to add to the language when there is an efficient and abundantly clear workaround available: a boolean variable.

pauldraper commented 4 years ago

On the other hand, a programmer familiar with every language but Python might stop and wonder what for/else means.

They have a lot of stuff to wonder about too, like significant whitespace(!!!).

With the abundantly clear workaround,

for root in roots:
    if src.path.startswith(root):
        modules.append(create_module(name = src.path[len(root):], file = src))
        break
else:
    fail("Module roots does not contain %s" % src.path, "srcs")

becomes

found  = False
for root in roots:
    if src.path.startswith(root):
        found = True
        modules.append(create_module(name = src.path[len(root):], file = src))
        break
if not found:
    fail("Module roots does not contain %s" % src.path, "srcs")

The Python is far more readable than the workaround.

alandonovan commented 4 years ago

I was about to say that the desugaring is incorrect: the found=True statement should be outside the if statement. But then I checked and found that, no, you are correct, which only proves my point: the semantics of 'else' are far from obvious, even to someone familiar with a dozen other languages. So again I say the desugared code is more readable, though obviously less concise.

Regarding significant whitespace, it may be surprising when you first see it, and it may be a cause of carelessly inserted mistakes, but its meaning is abundantly obvious.

laurentlb commented 4 years ago

Your rewrite with the found boolean seems more obvious to me. This feature is rarely found in other languages and it can create confusion.

I've migrated tons of files at Google from Python to Starlark, and this else pattern was very rarely used in practice. I really don't think we need this.

pauldraper commented 4 years ago

But then I checked and found that, no, you are correct, which only proves my point

Or rather it proves my point, that you would have desugared wrong and the original form was a more reliable to express a common sort of thing.

This feature is rarely found in other languages and it can create confusion.

And Skylark still chooses to have significant whitespaces.

It seems that presence of feature in other languages is not actually a criterion.

laurentlb commented 4 years ago

Whitespace is not a feature, it's the base of the language. You might say it was a mistake and another syntax would be better, but looking like Python was a design principle (https://github.com/bazelbuild/starlark#design-principles). That's not relevant to this thread.

There's also a desire to keep the number of features low and limit the number of concepts someone needs to know in order to read the code.

Or rather it proves my point, that you would have desugared wrong

The desugared form is obvious. Read the code, it does what it says. On the other hand, you wouldn't necessarily guess the meaning of else (Does it run if we didn't enter the first block, like in if? Actually, no).

It seems that presence of feature in other languages is not actually a criterion.

Familiarity is one of the goals.