Require braces on control flow statements (#683)

This implements #623

executable_semantics/syntax/parser.ypp

@@ -37,17 +37,8 @@
 // "inout" parameter passed to both the parser and the lexer.
 %param {Carbon::ParseAndLexContext& context}
 
-// The following shift-reduce conflicts are expected; any others should be
-// treated as errors:
-// - The "dangling else" ambiguity: `if (b) if (c) x = 1; else x = 2;`
-//   could parse as either `if (b) { if (c) x = 1; else x = 2;}` or
-//   `if (b) { if (c) x = 1; } else x = 2;`. Following C++, we want Carbon
-//   to choose the first option. Resolving this by restructuring the grammar
-//   would make it harder to read, and resolving it by assigning precedence to
-//   `if` and `else` could mask other ambiguities, especially if we allow
-//   `if`/`else` in expressions, so we allow Bison to resolve it through its
-//   default behavior of preferring to shift rather than reduce.
-%expect 1
+// No shift-reduce conflicts are expected.
+%expect 0
 
 // -----------------------------------------------------------------------------
 
@@ -95,7 +86,9 @@ void yy::parser::error(const location_type&, const std::string& message) {
 %type <Carbon::FunctionDefinition> function_definition
 %type <std::list<Carbon::Declaration>> declaration_list
 %type <const Carbon::Statement*> statement
+%type <const Carbon::Statement*> if_statement
 %type <const Carbon::Statement*> optional_else
+%type <const Carbon::Statement*> block
 %type <const Carbon::Statement*> statement_list
 %type <const Carbon::Expression*> expression
 %type <Carbon::GenericBinding> generic_binding
@@ -338,9 +331,9 @@ statement:
     { $$ = Carbon::Statement::MakeVariableDefinition(yylineno, $2, $4); }
 | expression ";"
     { $$ = Carbon::Statement::MakeExpressionStatement(yylineno, $1); }
-| IF "(" expression ")" statement optional_else
-    { $$ = Carbon::Statement::MakeIf(yylineno, $3, $5, $6); }
-| WHILE "(" expression ")" statement
+| if_statement
+    { $$ = $1; }
+| WHILE "(" expression ")" block
     { $$ = Carbon::Statement::MakeWhile(yylineno, $3, $5); }
 | BREAK ";"
     { $$ = Carbon::Statement::MakeBreak(yylineno); }
@@ -348,8 +341,8 @@ statement:
     { $$ = Carbon::Statement::MakeContinue(yylineno); }
 | RETURN expression ";"
     { $$ = Carbon::Statement::MakeReturn(yylineno, $2); }
-| "{" statement_list "}"
-    { $$ = Carbon::Statement::MakeBlock(yylineno, $2); }
+| block
+    { $$ = $1; }
 | MATCH "(" expression ")" "{" clause_list "}"
     { $$ = Carbon::Statement::MakeMatch(yylineno, $3, $6); }
 | CONTINUATION identifier statement
@@ -359,10 +352,17 @@ statement:
 | AWAIT ";"
     { $$ = Carbon::Statement::MakeAwait(yylineno); }
 ;
+if_statement:
+  IF "(" expression ")" block optional_else
+    { $$ = Carbon::Statement::MakeIf(yylineno, $3, $5, $6); }
+;
 optional_else:
   // Empty
     { $$ = 0; }
-| ELSE statement { $$ = $2; }
+| ELSE if_statement
+    { $$ = $2; }
+| ELSE block
+    { $$ = $2; }
 ;
 statement_list:
   // Empty
@@ -370,6 +370,10 @@ statement_list:
 | statement statement_list
     { $$ = Carbon::Statement::MakeSequence(yylineno, $1, $2); }
 ;
+block:
+  "{" statement_list "}"
+    { $$ = Carbon::Statement::MakeBlock(yylineno, $2); }
+;
 return_type:
   // Empty
     { $$ = Carbon::Expression::MakeTupleLiteral(yylineno, {}); }
@@ -403,8 +407,8 @@ deduced_params:
     { $$ = $2; }
 ;
 function_definition:
-  FN identifier deduced_params tuple return_type "{" statement_list "}"
-    { $$ = Carbon::FunctionDefinition(yylineno, $2, $3, $4, $5, $7); }
+  FN identifier deduced_params tuple return_type block
+    { $$ = Carbon::FunctionDefinition(yylineno, $2, $3, $4, $5, $6); }
 | FN identifier deduced_params tuple DBLARROW expression ";"
     {
       $$ = Carbon::FunctionDefinition(

executable_semantics/test_list.bzl

@@ -48,9 +48,12 @@ TEST_LIST = [
     "global_variable6",
     "global_variable7",
     "global_variable8",
-    "if1",
-    "if2",
-    "if3",
+    "if_else",
+    "if_else_if",
+    "if_else_if_else",
+    "if_false",
+    "if_nesting",
+    "if_true",
     "ignored_parameter",
     "invalid_char",
     "match_any_int",

executable_semantics/testdata/break1.carbon

@@ -5,10 +5,11 @@
 fn main() -> Int {
   var x: Int = 2;
   while (true) {
-    if (x == 0)
+    if (x == 0) {
       break;
-    else
+    } else {
       x = x - 1;
+    }
   }
   return x;
 }

executable_semantics/testdata/fun_recur.carbon

@@ -3,10 +3,11 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 fn f(x: Int) -> Int {
-  if (x == 0)
+  if (x == 0) {
     return x;
-  else
+  } else {
     return f(x - 1);
+  }
 }
 
 fn main() -> Int {

executable_semantics/testdata/if_else.carbon

@@ -0,0 +1,13 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+fn main() -> Int {
+  if (0 == 1) {
+    return 1;
+  } else {
+    return 0;
+  }
+
+  return 1;
+}

executable_semantics/testdata/if_else_if.carbon

@@ -0,0 +1,13 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+fn main() -> Int {
+  if (0 == 1) {
+    return 1;
+  } else if (0 == 0) {
+    return 0;
+  }
+
+  return 1;
+}

executable_semantics/testdata/if_else_if_else.carbon

@@ -0,0 +1,15 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+fn main() -> Int {
+  if (0 == 1) {
+    return 1;
+  } else if (0 == 2) {
+    return 2;
+  } else {
+    return 0;
+  }
+
+  return 1;
+}

executable_semantics/testdata/if2.carbon → executable_semantics/testdata/if_false.carbon

@@ -3,8 +3,9 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 fn main() -> Int {
-  if (0 == 1)
+  if (0 == 1) {
     return 1;
+  }
 
   return 0;
 }

executable_semantics/testdata/if_false.golden

@@ -0,0 +1 @@
+result: 0

executable_semantics/testdata/if3.carbon → executable_semantics/testdata/if_nesting.carbon

@@ -3,11 +3,13 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 fn main() -> Int {
-  if (0 == 0)
-    if (0 == 1)
+  if (0 == 0) {
+    if (0 == 1) {
       return 1;
-    else
+    } else {
       return 0;
+    }
+  }
 
   return 1;
 }

executable_semantics/testdata/if_nesting.golden

@@ -0,0 +1 @@
+result: 0

executable_semantics/testdata/if1.carbon → executable_semantics/testdata/if_true.carbon

@@ -3,8 +3,9 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 fn main() -> Int {
-  if (1 == 1)
+  if (1 == 1) {
     return 0;
+  }
 
   return 1;
 }

executable_semantics/testdata/if_true.golden

@@ -0,0 +1 @@
+result: 0

executable_semantics/testdata/while1.carbon

@@ -4,7 +4,8 @@
 
 fn main() -> Int {
   var x: auto = 2;
-  while (not (x == 0))
+  while (not (x == 0)) {
     x = x - 1;
+  }
   return x;
 }