Be more explicit about AST types (#921)

This was born out of wanting FunctionDeclaration to explicitly have a Block for a body, and became a bit more of specifying types around. Note this forces exec_program to generate a Block for print()'s body, which is probably more correct as now we can expect a standard FunctionDeclaration AST structure, even for the built-ins.

There is a syntactic change here: a continuation's body is now a Block, not just a Statement. I've added an example disallowed test case. I think this is more reasonable syntax.

Other than that, note that optional_else now generates a valid Block. This has me thinking about whether we can eliminate Sequence, but that seemed well out of scope for this.

executable_semantics/ast/declaration.h

@@ -90,7 +90,7 @@ class FunctionDeclaration : public Declaration {
                       Nonnull<TuplePattern*> param_pattern,
                       Nonnull<Pattern*> return_type,
                       bool is_omitted_return_type,
-                      std::optional<Nonnull<Statement*>> body)
+                      std::optional<Nonnull<Block*>> body)
       : Declaration(Kind::FunctionDeclaration, source_loc),
         name_(std::move(name)),
         deduced_parameters_(std::move(deduced_params)),
@@ -116,10 +116,8 @@ class FunctionDeclaration : public Declaration {
   auto is_omitted_return_type() const -> bool {
     return is_omitted_return_type_;
   }
-  auto body() const -> std::optional<Nonnull<const Statement*>> {
-    return body_;
-  }
-  auto body() -> std::optional<Nonnull<Statement*>> { return body_; }
+  auto body() const -> std::optional<Nonnull<const Block*>> { return body_; }
+  auto body() -> std::optional<Nonnull<Block*>> { return body_; }
 
  private:
   std::string name_;
@@ -127,7 +125,7 @@ class FunctionDeclaration : public Declaration {
   Nonnull<TuplePattern*> param_pattern_;
   Nonnull<Pattern*> return_type_;
   bool is_omitted_return_type_;
-  std::optional<Nonnull<Statement*>> body_;
+  std::optional<Nonnull<Block*>> body_;
 };
 
 class ClassDeclaration : public Declaration {

executable_semantics/ast/expression.cpp

@@ -30,7 +30,7 @@ auto ExpressionFromParenContents(
 
 auto TupleExpressionFromParenContents(
     Nonnull<Arena*> arena, SourceLocation source_loc,
-    const ParenContents<Expression>& paren_contents) -> Nonnull<Expression*> {
+    const ParenContents<Expression>& paren_contents) -> Nonnull<TupleLiteral*> {
   return arena->New<TupleLiteral>(source_loc, paren_contents.elements);
 }
 

executable_semantics/ast/expression.h

@@ -79,19 +79,6 @@ class Expression {
   std::optional<Nonnull<const Value*>> static_type_;
 };
 
-// Converts paren_contents to an Expression, interpreting the parentheses as
-// grouping if their contents permit that interpretation, or as forming a
-// tuple otherwise.
-auto ExpressionFromParenContents(
-    Nonnull<Arena*> arena, SourceLocation source_loc,
-    const ParenContents<Expression>& paren_contents) -> Nonnull<Expression*>;
-
-// Converts paren_contents to an Expression, interpreting the parentheses as
-// forming a tuple.
-auto TupleExpressionFromParenContents(
-    Nonnull<Arena*> arena, SourceLocation source_loc,
-    const ParenContents<Expression>& paren_contents) -> Nonnull<Expression*>;
-
 // A FieldInitializer represents the initialization of a single struct field.
 class FieldInitializer {
  public:
@@ -450,6 +437,19 @@ class IntrinsicExpression : public Expression {
   Intrinsic intrinsic_;
 };
 
+// Converts paren_contents to an Expression, interpreting the parentheses as
+// grouping if their contents permit that interpretation, or as forming a
+// tuple otherwise.
+auto ExpressionFromParenContents(
+    Nonnull<Arena*> arena, SourceLocation source_loc,
+    const ParenContents<Expression>& paren_contents) -> Nonnull<Expression*>;
+
+// Converts paren_contents to an Expression, interpreting the parentheses as
+// forming a tuple.
+auto TupleExpressionFromParenContents(
+    Nonnull<Arena*> arena, SourceLocation source_loc,
+    const ParenContents<Expression>& paren_contents) -> Nonnull<TupleLiteral*>;
+
 }  // namespace Carbon
 
 #endif  // EXECUTABLE_SEMANTICS_AST_EXPRESSION_H_

executable_semantics/ast/statement.cpp

@@ -62,10 +62,10 @@ void Statement::PrintDepth(int depth, llvm::raw_ostream& out) const {
     case Kind::If: {
       const auto& if_stmt = cast<If>(*this);
       out << "if (" << if_stmt.condition() << ")\n";
-      if_stmt.then_statement().PrintDepth(depth - 1, out);
-      if (if_stmt.else_statement()) {
+      if_stmt.then_block().PrintDepth(depth - 1, out);
+      if (if_stmt.else_block()) {
         out << "\nelse\n";
-        (*if_stmt.else_statement())->PrintDepth(depth - 1, out);
+        (*if_stmt.else_block())->PrintDepth(depth - 1, out);
       }
       break;
     }
@@ -97,8 +97,8 @@ void Statement::PrintDepth(int depth, llvm::raw_ostream& out) const {
       if (depth < 0 || depth > 1) {
         out << "\n";
       }
-      if (block.statement()) {
-        (*block.statement())->PrintDepth(depth, out);
+      if (block.sequence()) {
+        (*block.sequence())->PrintDepth(depth, out);
         if (depth < 0 || depth > 1) {
           out << "\n";
         }

executable_semantics/ast/statement.h

@@ -60,6 +60,48 @@ class Statement {
   SourceLocation source_loc_;
 };
 
+class Sequence : public Statement {
+ public:
+  Sequence(SourceLocation source_loc, Nonnull<Statement*> statement,
+           std::optional<Nonnull<Statement*>> next)
+      : Statement(Kind::Sequence, source_loc),
+        statement_(statement),
+        next_(next) {}
+
+  static auto classof(const Statement* stmt) -> bool {
+    return stmt->kind() == Kind::Sequence;
+  }
+
+  auto statement() const -> const Statement& { return *statement_; }
+  auto statement() -> Statement& { return *statement_; }
+  auto next() const -> std::optional<Nonnull<const Statement*>> {
+    return next_;
+  }
+  auto next() -> std::optional<Nonnull<Statement*>> { return next_; }
+
+ private:
+  Nonnull<Statement*> statement_;
+  std::optional<Nonnull<Statement*>> next_;
+};
+
+class Block : public Statement {
+ public:
+  Block(SourceLocation source_loc, std::optional<Nonnull<Sequence*>> sequence)
+      : Statement(Kind::Block, source_loc), sequence_(sequence) {}
+
+  static auto classof(const Statement* stmt) -> bool {
+    return stmt->kind() == Kind::Block;
+  }
+
+  auto sequence() const -> std::optional<Nonnull<const Sequence*>> {
+    return sequence_;
+  }
+  auto sequence() -> std::optional<Nonnull<Sequence*>> { return sequence_; }
+
+ private:
+  std::optional<Nonnull<Sequence*>> sequence_;
+};
+
 class ExpressionStatement : public Statement {
  public:
   ExpressionStatement(SourceLocation source_loc,
@@ -123,12 +165,11 @@ class VariableDefinition : public Statement {
 class If : public Statement {
  public:
   If(SourceLocation source_loc, Nonnull<Expression*> condition,
-     Nonnull<Statement*> then_statement,
-     std::optional<Nonnull<Statement*>> else_statement)
+     Nonnull<Block*> then_block, std::optional<Nonnull<Block*>> else_block)
       : Statement(Kind::If, source_loc),
         condition_(condition),
-        then_statement_(then_statement),
-        else_statement_(else_statement) {}
+        then_block_(then_block),
+        else_block_(else_block) {}
 
   static auto classof(const Statement* stmt) -> bool {
     return stmt->kind() == Kind::If;
@@ -136,19 +177,17 @@ class If : public Statement {
 
   auto condition() const -> const Expression& { return *condition_; }
   auto condition() -> Expression& { return *condition_; }
-  auto then_statement() const -> const Statement& { return *then_statement_; }
-  auto then_statement() -> Statement& { return *then_statement_; }
-  auto else_statement() const -> std::optional<Nonnull<const Statement*>> {
-    return else_statement_;
-  }
-  auto else_statement() -> std::optional<Nonnull<Statement*>> {
-    return else_statement_;
+  auto then_block() const -> const Block& { return *then_block_; }
+  auto then_block() -> Block& { return *then_block_; }
+  auto else_block() const -> std::optional<Nonnull<const Block*>> {
+    return else_block_;
   }
+  auto else_block() -> std::optional<Nonnull<Block*>> { return else_block_; }
 
  private:
   Nonnull<Expression*> condition_;
-  Nonnull<Statement*> then_statement_;
-  std::optional<Nonnull<Statement*>> else_statement_;
+  Nonnull<Block*> then_block_;
+  std::optional<Nonnull<Block*>> else_block_;
 };
 
 class Return : public Statement {
@@ -189,52 +228,10 @@ class Return : public Statement {
   std::optional<Nonnull<const FunctionDeclaration*>> function_;
 };
 
-class Sequence : public Statement {
- public:
-  Sequence(SourceLocation source_loc, Nonnull<Statement*> statement,
-           std::optional<Nonnull<Statement*>> next)
-      : Statement(Kind::Sequence, source_loc),
-        statement_(statement),
-        next_(next) {}
-
-  static auto classof(const Statement* stmt) -> bool {
-    return stmt->kind() == Kind::Sequence;
-  }
-
-  auto statement() const -> const Statement& { return *statement_; }
-  auto statement() -> Statement& { return *statement_; }
-  auto next() const -> std::optional<Nonnull<const Statement*>> {
-    return next_;
-  }
-  auto next() -> std::optional<Nonnull<Statement*>> { return next_; }
-
- private:
-  Nonnull<Statement*> statement_;
-  std::optional<Nonnull<Statement*>> next_;
-};
-
-class Block : public Statement {
- public:
-  Block(SourceLocation source_loc, std::optional<Nonnull<Statement*>> statement)
-      : Statement(Kind::Block, source_loc), statement_(statement) {}
-
-  static auto classof(const Statement* stmt) -> bool {
-    return stmt->kind() == Kind::Block;
-  }
-
-  auto statement() const -> std::optional<Nonnull<const Statement*>> {
-    return statement_;
-  }
-  auto statement() -> std::optional<Nonnull<Statement*>> { return statement_; }
-
- private:
-  std::optional<Nonnull<Statement*>> statement_;
-};
-
 class While : public Statement {
  public:
   While(SourceLocation source_loc, Nonnull<Expression*> condition,
-        Nonnull<Statement*> body)
+        Nonnull<Block*> body)
       : Statement(Kind::While, source_loc),
         condition_(condition),
         body_(body) {}
@@ -245,12 +242,12 @@ class While : public Statement {
 
   auto condition() const -> const Expression& { return *condition_; }
   auto condition() -> Expression& { return *condition_; }
-  auto body() const -> const Statement& { return *body_; }
-  auto body() -> Statement& { return *body_; }
+  auto body() const -> const Block& { return *body_; }
+  auto body() -> Block& { return *body_; }
 
  private:
   Nonnull<Expression*> condition_;
-  Nonnull<Statement*> body_;
+  Nonnull<Block*> body_;
 };
 
 class Break : public Statement {
@@ -352,7 +349,7 @@ class Match : public Statement {
 class Continuation : public Statement {
  public:
   Continuation(SourceLocation source_loc, std::string continuation_variable,
-               Nonnull<Statement*> body)
+               Nonnull<Block*> body)
       : Statement(Kind::Continuation, source_loc),
         continuation_variable_(std::move(continuation_variable)),
         body_(body) {}
@@ -364,12 +361,12 @@ class Continuation : public Statement {
   auto continuation_variable() const -> const std::string& {
     return continuation_variable_;
   }
-  auto body() const -> const Statement& { return *body_; }
-  auto body() -> Statement& { return *body_; }
+  auto body() const -> const Block& { return *body_; }
+  auto body() -> Block& { return *body_; }
 
  private:
   std::string continuation_variable_;
-  Nonnull<Statement*> body_;
+  Nonnull<Block*> body_;
 };
 
 // A run statement.

executable_semantics/interpreter/exec_program.cpp

@@ -22,10 +22,15 @@ static void AddIntrinsics(Nonnull<Arena*> arena,
       source_loc, "format_str",
       arena->New<ExpressionPattern>(
           arena->New<StringTypeLiteral>(source_loc)))};
-  auto print_return = arena->New<Return>(
+  auto print_return = arena->New<Block>(
       source_loc,
-      arena->New<IntrinsicExpression>(IntrinsicExpression::Intrinsic::Print),
-      false);
+      arena->New<Sequence>(
+          source_loc,
+          arena->New<Return>(source_loc,
+                             arena->New<IntrinsicExpression>(
+                                 IntrinsicExpression::Intrinsic::Print),
+                             false),
+          std::nullopt));
   auto print = arena->New<FunctionDeclaration>(
       source_loc, "Print", std::vector<GenericBinding>(),
       arena->New<TuplePattern>(source_loc, print_params),

executable_semantics/interpreter/interpreter.cpp

@@ -859,9 +859,9 @@ auto Interpreter::StepStmt() -> Transition {
     case Statement::Kind::Block: {
       if (act->pos() == 0) {
         const Block& block = cast<Block>(stmt);
-        if (block.statement()) {
+        if (block.sequence()) {
           act->StartScope(Scope(CurrentEnv()));
-          return Spawn{arena_->New<StatementAction>(*block.statement())};
+          return Spawn{arena_->New<StatementAction>(*block.sequence())};
         } else {
           return Done{};
         }
@@ -937,13 +937,13 @@ auto Interpreter::StepStmt() -> Transition {
           //      S, H}
           // -> { { then_stmt :: C, E, F } :: S, H}
           return Delegate{
-              arena_->New<StatementAction>(&cast<If>(stmt).then_statement())};
-        } else if (cast<If>(stmt).else_statement()) {
+              arena_->New<StatementAction>(&cast<If>(stmt).then_block())};
+        } else if (cast<If>(stmt).else_block()) {
           //    { {false :: if ([]) then_stmt else else_stmt :: C, E, F} ::
           //      S, H}
           // -> { { else_stmt :: C, E, F } :: S, H}
           return Delegate{
-              arena_->New<StatementAction>(*cast<If>(stmt).else_statement())};
+              arena_->New<StatementAction>(*cast<If>(stmt).else_block())};
         } else {
           return Done{};
         }

executable_semantics/interpreter/resolve_control_flow.cpp

@@ -45,9 +45,9 @@ static void ResolveControlFlow(
       return;
     case Statement::Kind::If: {
       auto& if_stmt = cast<If>(*statement);
-      ResolveControlFlow(&if_stmt.then_statement(), function, loop);
-      if (if_stmt.else_statement().has_value()) {
-        ResolveControlFlow(*if_stmt.else_statement(), function, loop);
+      ResolveControlFlow(&if_stmt.then_block(), function, loop);
+      if (if_stmt.else_block().has_value()) {
+        ResolveControlFlow(*if_stmt.else_block(), function, loop);
       }
       return;
     }
@@ -61,8 +61,8 @@ static void ResolveControlFlow(
     }
     case Statement::Kind::Block: {
       auto& block = cast<Block>(*statement);
-      if (block.statement().has_value()) {
-        ResolveControlFlow(*block.statement(), function, loop);
+      if (block.sequence().has_value()) {
+        ResolveControlFlow(*block.sequence(), function, loop);
       }
       return;
     }

executable_semantics/interpreter/type_checker.cpp

@@ -868,8 +868,8 @@ auto TypeChecker::TypeCheckStmt(Nonnull<Statement*> s, TypeEnv types,
       return TCResult(types);
     case Statement::Kind::Block: {
       auto& block = cast<Block>(*s);
-      if (block.statement()) {
-        TypeCheckStmt(*block.statement(), types, values, return_type_context);
+      if (block.sequence()) {
+        TypeCheckStmt(*block.sequence(), types, values, return_type_context);
         return TCResult(types);
       } else {
         return TCResult(types);
@@ -911,10 +911,9 @@ auto TypeChecker::TypeCheckStmt(Nonnull<Statement*> s, TypeEnv types,
       TypeCheckExp(&if_stmt.condition(), types, values);
       ExpectType(s->source_loc(), "condition of `if`", arena_->New<BoolType>(),
                  &if_stmt.condition().static_type());
-      TypeCheckStmt(&if_stmt.then_statement(), types, values,
-                    return_type_context);
-      if (if_stmt.else_statement()) {
-        TypeCheckStmt(*if_stmt.else_statement(), types, values,
+      TypeCheckStmt(&if_stmt.then_block(), types, values, return_type_context);
+      if (if_stmt.else_block()) {
+        TypeCheckStmt(*if_stmt.else_block(), types, values,
                       return_type_context);
       }
       return TCResult(types);
@@ -1005,13 +1004,12 @@ void TypeChecker::ExpectReturnOnAllPaths(
       return;
     }
     case Statement::Kind::Block:
-      ExpectReturnOnAllPaths(cast<Block>(*stmt).statement(),
-                             stmt->source_loc());
+      ExpectReturnOnAllPaths(cast<Block>(*stmt).sequence(), stmt->source_loc());
       return;
     case Statement::Kind::If: {
       auto& if_stmt = cast<If>(*stmt);
-      ExpectReturnOnAllPaths(&if_stmt.then_statement(), stmt->source_loc());
-      ExpectReturnOnAllPaths(if_stmt.else_statement(), stmt->source_loc());
+      ExpectReturnOnAllPaths(&if_stmt.then_block(), stmt->source_loc());
+      ExpectReturnOnAllPaths(if_stmt.else_block(), stmt->source_loc());
       return;
     }
     case Statement::Kind::Return:

executable_semantics/syntax/parser.ypp

@@ -101,13 +101,13 @@
 %type <Nonnull<FunctionDeclaration*>> function_declaration
 %type <std::vector<Nonnull<Declaration*>>> declaration_list
 %type <Nonnull<Statement*>> statement
-%type <Nonnull<Statement*>> if_statement
-%type <std::optional<Nonnull<Statement*>>> optional_else
+%type <Nonnull<If*>> if_statement
+%type <std::optional<Nonnull<Block*>>> optional_else
 %type <std::pair<Nonnull<Expression*>, bool>> return_expression
-%type <Nonnull<Statement*>> nonempty_block
-%type <Nonnull<Statement*>> block
-%type <Nonnull<Statement*>> nonempty_statement_list
-%type <std::optional<Nonnull<Statement*>>> statement_list
+%type <Nonnull<Block*>> nonempty_block
+%type <Nonnull<Block*>> block
+%type <Nonnull<Sequence*>> nonempty_statement_list
+%type <std::optional<Nonnull<Sequence*>>> statement_list
 %type <Nonnull<Expression*>> expression
 %type <GenericBinding> generic_binding
 %type <std::vector<GenericBinding>> deduced_params
@@ -116,11 +116,11 @@
 %type <Nonnull<Pattern*>> non_expression_pattern
 %type <std::pair<Nonnull<Expression*>, bool>> return_type
 %type <Nonnull<Expression*>> paren_expression
-%type <Nonnull<Expression*>> struct_literal
+%type <Nonnull<StructLiteral*>> struct_literal
 %type <std::vector<FieldInitializer>> struct_literal_contents
-%type <Nonnull<Expression*>> struct_type_literal
+%type <Nonnull<StructTypeLiteral*>> struct_type_literal
 %type <std::vector<FieldInitializer>> struct_type_literal_contents
-%type <Nonnull<Expression*>> tuple
+%type <Nonnull<TupleLiteral*>> tuple
 %type <std::optional<std::string>> binding_lhs
 %type <Nonnull<BindingPattern*>> variable_declaration
 %type <Nonnull<Member*>> member
@@ -577,7 +577,7 @@ statement:
 | MATCH LEFT_PARENTHESIS expression RIGHT_PARENTHESIS LEFT_CURLY_BRACE
   clause_list RIGHT_CURLY_BRACE
     { $$ = arena->New<Match>(context.source_loc(), $3, $6); }
-| CONTINUATION identifier statement
+| CONTINUATION identifier block
     { $$ = arena->New<Continuation>(context.source_loc(), $2, $3); }
 | RUN expression SEMICOLON
     { $$ = arena->New<Run>(context.source_loc(), $2); }
@@ -592,7 +592,11 @@ optional_else:
   // Empty
     { $$ = std::nullopt; }
 | ELSE if_statement
-    { $$ = $2; }
+    {
+      $$ = arena->New<Block>(
+          context.source_loc(),
+          arena->New<Sequence>(context.source_loc(), $2, std::nullopt));
+    }
 | ELSE block
     { $$ = $2; }
 ;

executable_semantics/testdata/experimental_continuation/fail_continuation_syntax.carbon

@@ -0,0 +1,19 @@
+// 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
+//
+// RUN: not executable_semantics %s 2>&1 | \
+// RUN:   FileCheck --match-full-lines --allow-unused-prefixes=false %s
+// RUN: not executable_semantics --trace %s 2>&1 | \
+// RUN:   FileCheck --match-full-lines --allow-unused-prefixes %s
+// AUTOUPDATE: executable_semantics %s
+// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/experimental_continuation/fail_continuation_syntax.carbon:16: syntax error, unexpected identifier, expecting LEFT_CURLY_BRACE
+
+package ExecutableSemanticsTest api;
+
+fn main() -> i32 {
+  var x: i32 = 0;
+  __continuation k x = 3;
+  __run k;
+  return x;
+}