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

Always require brackets {} when using if/else/when/for etc. #4294

Closed
momumi opened this issue Jan 26, 2020 · 11 comments
Closed

Always require brackets {} when using if/else/when/for etc. #4294

momumi opened this issue Jan 26, 2020 · 11 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@momumi
Copy link
Contributor

momumi commented Jan 26, 2020

Given that zig values only one way to do things, it's very surprising to me that braces are optional for one-line if/while/for expressions. Optional braces was also the cause of the famous goto fail bug.

If braces are always required, you can still write clean one-line expressions:

if (cond1) { doStuff(); } else { doOtherThing(); }
if (cond2) { doStuff(); } else { doOtherThing(); }
if (cond3) { doStuff(); } else { doOtherThing(); }

while (cond) { doWork(); }

for (things) |item| { process(item); }

Tenary if/else still works without braces too: const z = if (cond) x else y;

Pros

Cons

  • Adds one extra line in some cases

Zen

This change aligns with the Zen of zig:

  • ✅ Communicate intent precisely.
  • ✅ Edge cases matter.
  • ✅ Favor reading code over writing code.
  • ✅ Only one obvious way to do things.
  • ✅ Runtime crashes are better than bugs.
  • ✅ Compile errors are better than runtime crashes.
  • ✅ Incremental improvements. (easier to add more code to the if statement later)
  • (?) Avoid local maximums.
  • ✅ Reduce the amount one must remember. (won't forget to add braces)
  • ✅ Minimize energy spent on coding style.
  • (?) Together we serve end users.
@daurnimator daurnimator added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Jan 26, 2020
@fengb
Copy link
Contributor

fengb commented Jan 26, 2020

Odin allows an explicit one line no bracket with the do keyword.

However I don’t think status quo is that bad because zig fmt makes the error really obvious.

@momumi
Copy link
Contributor Author

momumi commented Jan 26, 2020

zig fmt makes the error really obvious.

Not in all cases, if you write something like this:

if (match_cond)
    x.removeThing(y);
    x.swap(z);
x.add(stuff);

it will silently be transformed into this:

if (match_cond)
    x.removeThing(y);
x.swap(z);
x.add(stuff);

which still looks like correct code.

@mogud
Copy link
Contributor

mogud commented Jan 27, 2020

In addition with this proposal, parentheses are not needed after if/while/for.

@ManDeJan
Copy link

I think being able to omit braces makes sense. I don't see braces as part of the if/for/while syntax but as part of the expression body. Braces are just a way to combine multiple statements into one block statement. You can make new blocks everywhere you can make a statement so to me it makes sense that you can also omit a block anywhere you want to make a statement.

@andrewrk andrewrk added this to the 0.7.0 milestone Jan 27, 2020
@Sobeston
Copy link
Sponsor Contributor

I think that this is silly - there are blocks everywhere in zig code. Enforcing braces on every if, else, orelse, while, for, etc will make code longer and potentially harder to read.

Perhaps zig fmt should transform this:

if (match_cond)
    x.removeThing(y);
    x.swap(z);
x.add(stuff);

into

if (match_cond) x.removeThing(y);
x.swap(z);
x.add(stuff);

or

if (match_cond)
    x.removeThing(y);

x.swap(z);
x.add(stuff);

, perhaps depending on line length, to remove some potential for human error here.

I have written very clear code that would be made less clear if I was required to use braces.

@Sobeston
Copy link
Sponsor Contributor

Additionally, this proposal differentiates between normal if statements and ternary-style if statements... I'm not aware of the difference in zig.
Turning these into two separate things complicates the language.
What difference do you want there to be between the two, and why separate them?

What about labelled blocks?

I also find the inclusion of all of these ticks for the zen of zig questionable - arguing that a proposal fits many of a vague set of outlines that are philosophical in nature isn't a sound argument.

@momumi
Copy link
Contributor Author

momumi commented Jan 28, 2020

Additionally, this proposal differentiates between normal if statements and ternary-style if statements... I'm not aware of the difference in zig.

Top level if statements are already treated differently since they allow expressions without a semicolon, and they also allow assignment:

// Can write block-level expressions like this if they return void
( {} );           // okay, need a semicolon

// This is an if expression that returns void
( if (true) {} ); // if expression

// However, if we remove the (), it becomes an if statement:
if (true) {}      // if statement, doesn't require semicolon here

// This needs a semicolon because it can't be parsed as an if statement
if (true) ( {} );     // if expression that returns void

// If statements allow assignment which isn't an expression in zig, it's a type of statement
if (true) w = 1;      // okay in if statement
if (true) (w = 1);    // error in if expression

You can make new blocks everywhere you can make a statement so to me it makes sense that you can also omit a block anywhere you want to make a statement.

That is the case in C, but in zig I think they behave more like:

IfStatement <- if '(' expr ')' voidLikeExpression ';'
            |  if '(' expr ')' BlockStatement

For example:

if (true) { if (true) {} }     // okay
if (true) if (true) {}         // error when we removed the braces (valid in C though)
if (true) if (true) {};        // okay
if (true) { const x: u8 = 1 }; // okay
if (true) const x: u8 = 1;     // error
if (true) x = 1;               // okay
while (running) switch (getState()) { .... }    // error

Anyway, simplifying the grammar is not the main point, it's to:

  • Help prevent "goto fail" bugs
  • Minimize energy spent on coding style.

Perhaps zig fmt should transform this

If we do go this route, I think it would be better if zig fmt requested user intervention. Syntactically the code is unambiguous, but the formatting suggests that the user intended both statements to be inside the if statement body. Silently reformatting doesn't seem like the best option.

This is similar to the tabs vs spaces style issue, so consistence is the main thing. We could choose to enforce that one-line if statements can never use braces, but I think that would be the harder option to enforce.

I don't mean to start another code formatting holy war, it just seems that optional braces are a strange design decision given zig's stance on other issues (tabs vs spaces, one way to do things, etc.).

@ghost
Copy link

ghost commented Feb 13, 2020

FWIW, this proposal is quite aligned with my proposals #4412 and #4285 .

This way, an identifier in front of the block scope could always be treated as a label. I think such syntax would be unambiguous for the compiler, and quite readable with syntax highlighting.

const x = if(true) blocklabel { // no need for keyword `seqblk` as in #4412. Can just use `if`
  const y = 1;
  break blocklabel, y+1;
}

And with the whole AConf literal approach( issue 4285 ) , there would be uniform syntax for stuff like this

while(cond) : (updateCond()) label .{
// initializing an imagined `WhileBlockConfiguration` struct
// no indentation to separate it from the while body
.optimizationTarget = .ReleaseFast,
.someOtherOption = .Option1
},{
   // while body here
}

if(true) .{.timeConst = true},{
  // use case, constant time crypto #1776
}

const S = struct .{
.isPacked = true,
.implementationTargets = .{Interface1, Interface2},
},{
   // struct body
};

@BarabasGitHub
Copy link
Contributor

I very much support having only one way to do things. Including formatting, even if it's not necessarily my preferred way of doing things. It makes it easier to read and modify code.

@ghost
Copy link

ghost commented Apr 15, 2020

@ManDeJan I disagree: You could say the same about the condition of an if statement. It's an expression. Nowhere else is a zig user forced to use () around an expression. And I think it even makes more sense to remove those: You could think of an if statement containing a block at the end, not an expr. But putting () around an expression is no grammatical object itself, it's just to specify precedence.

@andrewrk
Copy link
Member

Note that this code:

if (match_cond)
    x.removeThing(y);
    x.swap(z);

will be rejected by both the compiler and zig fmt, after #35 is implemented, which has been planned since 2015.

@momumi thanks for the proposal. I did seriously consider doing this early on in Zig's syntax. Ultimately, had to go with the parentheses to make the syntax work. I'm confident in this decision.

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