Verify that provision/omission of return types matches (#717)

This is finishing #678

executable_semantics/interpreter/typecheck.cpp

@@ -24,8 +24,8 @@ using llvm::dyn_cast;
 
 namespace Carbon {
 
-void ExpectType(int line_num, const std::string& context, const Value* expected,
-                const Value* actual) {
+static void ExpectType(int line_num, const std::string& context,
+                       const Value* expected, const Value* actual) {
   if (!TypeEqual(expected, actual)) {
     FATAL_COMPILATION_ERROR(line_num) << "type error in " << context << "\n"
                                       << "expected: " << *expected << "\n"
@@ -33,8 +33,8 @@ void ExpectType(int line_num, const std::string& context, const Value* expected,
   }
 }
 
-void ExpectPointerType(int line_num, const std::string& context,
-                       const Value* actual) {
+static void ExpectPointerType(int line_num, const std::string& context,
+                              const Value* actual) {
   if (actual->Tag() != Value::Kind::PointerType) {
     FATAL_COMPILATION_ERROR(line_num) << "type error in " << context << "\n"
                                       << "expected a pointer type\n"
@@ -43,7 +43,7 @@ void ExpectPointerType(int line_num, const std::string& context,
 }
 
 // Reify type to type expression.
-auto ReifyType(const Value* t, int line_num) -> const Expression* {
+static auto ReifyType(const Value* t, int line_num) -> const Expression* {
   switch (t->Tag()) {
     case Value::Kind::IntType:
       return Expression::MakeIntTypeLiteral(0);
@@ -92,8 +92,8 @@ auto ReifyType(const Value* t, int line_num) -> const Expression* {
 // inside the argument type.
 // The `deduced` parameter is an accumulator, that is, it holds the
 // results so-far.
-auto ArgumentDeduction(int line_num, TypeEnv deduced, const Value* param,
-                       const Value* arg) -> TypeEnv {
+static auto ArgumentDeduction(int line_num, TypeEnv deduced, const Value* param,
+                              const Value* arg) -> TypeEnv {
   switch (param->Tag()) {
     case Value::Kind::VariableType: {
       const auto& var_type = cast<VariableType>(*param);
@@ -175,7 +175,7 @@ auto ArgumentDeduction(int line_num, TypeEnv deduced, const Value* param,
   }
 }
 
-auto Substitute(TypeEnv dict, const Value* type) -> const Value* {
+static auto Substitute(TypeEnv dict, const Value* type) -> const Value* {
   switch (type->Tag()) {
     case Value::Kind::VariableType: {
       std::optional<const Value*> t =
@@ -613,12 +613,13 @@ auto TypeCheckPattern(const Pattern* p, TypeEnv types, Env values,
   }
 }
 
-auto TypecheckCase(const Value* expected, const Pattern* pat,
-                   const Statement* body, TypeEnv types, Env values,
-                   const Value*& ret_type)
+static auto TypecheckCase(const Value* expected, const Pattern* pat,
+                          const Statement* body, TypeEnv types, Env values,
+                          const Value*& ret_type, bool is_omitted_ret_type)
     -> std::pair<const Pattern*, const Statement*> {
   auto pat_res = TypeCheckPattern(pat, types, values, expected);
-  auto res = TypeCheckStmt(body, pat_res.types, values, ret_type);
+  auto res =
+      TypeCheckStmt(body, pat_res.types, values, ret_type, is_omitted_ret_type);
   return std::make_pair(pat, res.stmt);
 }
 
@@ -630,7 +631,8 @@ auto TypecheckCase(const Value* expected, const Pattern* pat,
 // If the return type is "auto", then the return type is inferred from
 // the first return statement.
 auto TypeCheckStmt(const Statement* s, TypeEnv types, Env values,
-                   const Value*& ret_type) -> TCStatement {
+                   const Value*& ret_type, bool is_omitted_ret_type)
+    -> TCStatement {
   if (!s) {
     return TCStatement(s, types);
   }
@@ -642,8 +644,9 @@ auto TypeCheckStmt(const Statement* s, TypeEnv types, Env values,
           global_arena
               ->New<std::list<std::pair<const Pattern*, const Statement*>>>();
       for (auto& clause : *s->GetMatch().clauses) {
-        new_clauses->push_back(TypecheckCase(
-            res_type, clause.first, clause.second, types, values, ret_type));
+        new_clauses->push_back(TypecheckCase(res_type, clause.first,
+                                             clause.second, types, values,
+                                             ret_type, is_omitted_ret_type));
       }
       const Statement* new_s =
           Statement::MakeMatch(s->line_num, res.exp, new_clauses);
@@ -653,8 +656,8 @@ auto TypeCheckStmt(const Statement* s, TypeEnv types, Env values,
       auto cnd_res = TypeCheckExp(s->GetWhile().cond, types, values);
       ExpectType(s->line_num, "condition of `while`",
                  global_arena->New<BoolType>(), cnd_res.type);
-      auto body_res =
-          TypeCheckStmt(s->GetWhile().body, types, values, ret_type);
+      auto body_res = TypeCheckStmt(s->GetWhile().body, types, values, ret_type,
+                                    is_omitted_ret_type);
       auto new_s =
           Statement::MakeWhile(s->line_num, cnd_res.exp, body_res.stmt);
       return TCStatement(new_s, types);
@@ -663,8 +666,8 @@ auto TypeCheckStmt(const Statement* s, TypeEnv types, Env values,
     case StatementKind::Continue:
       return TCStatement(s, types);
     case StatementKind::Block: {
-      auto stmt_res =
-          TypeCheckStmt(s->GetBlock().stmt, types, values, ret_type);
+      auto stmt_res = TypeCheckStmt(s->GetBlock().stmt, types, values, ret_type,
+                                    is_omitted_ret_type);
       return TCStatement(Statement::MakeBlock(s->line_num, stmt_res.stmt),
                          types);
     }
@@ -678,11 +681,11 @@ auto TypeCheckStmt(const Statement* s, TypeEnv types, Env values,
       return TCStatement(new_s, lhs_res.types);
     }
     case StatementKind::Sequence: {
-      auto stmt_res =
-          TypeCheckStmt(s->GetSequence().stmt, types, values, ret_type);
+      auto stmt_res = TypeCheckStmt(s->GetSequence().stmt, types, values,
+                                    ret_type, is_omitted_ret_type);
       auto types2 = stmt_res.types;
-      auto next_res =
-          TypeCheckStmt(s->GetSequence().next, types2, values, ret_type);
+      auto next_res = TypeCheckStmt(s->GetSequence().next, types2, values,
+                                    ret_type, is_omitted_ret_type);
       auto types3 = next_res.types;
       return TCStatement(
           Statement::MakeSequence(s->line_num, stmt_res.stmt, next_res.stmt),
@@ -706,16 +709,17 @@ auto TypeCheckStmt(const Statement* s, TypeEnv types, Env values,
       auto cnd_res = TypeCheckExp(s->GetIf().cond, types, values);
       ExpectType(s->line_num, "condition of `if`",
                  global_arena->New<BoolType>(), cnd_res.type);
-      auto thn_res =
-          TypeCheckStmt(s->GetIf().then_stmt, types, values, ret_type);
-      auto els_res =
-          TypeCheckStmt(s->GetIf().else_stmt, types, values, ret_type);
+      auto thn_res = TypeCheckStmt(s->GetIf().then_stmt, types, values,
+                                   ret_type, is_omitted_ret_type);
+      auto els_res = TypeCheckStmt(s->GetIf().else_stmt, types, values,
+                                   ret_type, is_omitted_ret_type);
       auto new_s = Statement::MakeIf(s->line_num, cnd_res.exp, thn_res.stmt,
                                      els_res.stmt);
       return TCStatement(new_s, types);
     }
     case StatementKind::Return: {
-      auto res = TypeCheckExp(s->GetReturn().exp, types, values);
+      const auto& ret = s->GetReturn();
+      auto res = TypeCheckExp(ret.exp, types, values);
       if (ret_type->Tag() == Value::Kind::AutoType) {
         // The following infers the return type from the first 'return'
         // statement. This will get more difficult with subtyping, when we
@@ -724,13 +728,19 @@ auto TypeCheckStmt(const Statement* s, TypeEnv types, Env values,
       } else {
         ExpectType(s->line_num, "return", ret_type, res.type);
       }
-      return TCStatement(Statement::MakeReturn(s->line_num, res.exp,
-                                               s->GetReturn().is_omitted_exp),
-                         types);
+      if (ret.is_omitted_exp != is_omitted_ret_type) {
+        FATAL_COMPILATION_ERROR(s->line_num)
+            << *s << " should" << (is_omitted_ret_type ? " not" : "")
+            << " provide a return value, to match the function's signature.";
+      }
+      return TCStatement(
+          Statement::MakeReturn(s->line_num, res.exp, ret.is_omitted_exp),
+          types);
     }
     case StatementKind::Continuation: {
       TCStatement body_result =
-          TypeCheckStmt(s->GetContinuation().body, types, values, ret_type);
+          TypeCheckStmt(s->GetContinuation().body, types, values, ret_type,
+                        is_omitted_ret_type);
       const Statement* new_continuation = Statement::MakeContinuation(
           s->line_num, s->GetContinuation().continuation_variable,
           body_result.stmt);
@@ -754,15 +764,16 @@ auto TypeCheckStmt(const Statement* s, TypeEnv types, Env values,
   }  // switch
 }
 
-auto CheckOrEnsureReturn(const Statement* stmt, bool void_return, int line_num)
-    -> const Statement* {
+static auto CheckOrEnsureReturn(const Statement* stmt, bool omitted_ret_type,
+                                int line_num) -> const Statement* {
   if (!stmt) {
-    if (void_return) {
+    if (omitted_ret_type) {
       return Statement::MakeReturn(line_num, nullptr,
                                    /*is_omitted_exp=*/true);
     } else {
       FATAL_COMPILATION_ERROR(line_num)
-          << "control-flow reaches end of non-void function without a return";
+          << "control-flow reaches end of function that provides a `->` return "
+             "type without reaching a return statement";
     }
   }
   switch (stmt->tag()) {
@@ -772,7 +783,8 @@ auto CheckOrEnsureReturn(const Statement* stmt, bool void_return, int line_num)
               ->New<std::list<std::pair<const Pattern*, const Statement*>>>();
       for (auto i = stmt->GetMatch().clauses->begin();
            i != stmt->GetMatch().clauses->end(); ++i) {
-        auto s = CheckOrEnsureReturn(i->second, void_return, stmt->line_num);
+        auto s =
+            CheckOrEnsureReturn(i->second, omitted_ret_type, stmt->line_num);
         new_clauses->push_back(std::make_pair(i->first, s));
       }
       return Statement::MakeMatch(stmt->line_num, stmt->GetMatch().exp,
@@ -780,14 +792,15 @@ auto CheckOrEnsureReturn(const Statement* stmt, bool void_return, int line_num)
     }
     case StatementKind::Block:
       return Statement::MakeBlock(
-          stmt->line_num, CheckOrEnsureReturn(stmt->GetBlock().stmt,
-                                              void_return, stmt->line_num));
+          stmt->line_num,
+          CheckOrEnsureReturn(stmt->GetBlock().stmt, omitted_ret_type,
+                              stmt->line_num));
     case StatementKind::If:
       return Statement::MakeIf(
           stmt->line_num, stmt->GetIf().cond,
-          CheckOrEnsureReturn(stmt->GetIf().then_stmt, void_return,
+          CheckOrEnsureReturn(stmt->GetIf().then_stmt, omitted_ret_type,
                               stmt->line_num),
-          CheckOrEnsureReturn(stmt->GetIf().else_stmt, void_return,
+          CheckOrEnsureReturn(stmt->GetIf().else_stmt, omitted_ret_type,
                               stmt->line_num));
     case StatementKind::Return:
       return stmt;
@@ -795,10 +808,10 @@ auto CheckOrEnsureReturn(const Statement* stmt, bool void_return, int line_num)
       if (stmt->GetSequence().next) {
         return Statement::MakeSequence(
             stmt->line_num, stmt->GetSequence().stmt,
-            CheckOrEnsureReturn(stmt->GetSequence().next, void_return,
+            CheckOrEnsureReturn(stmt->GetSequence().next, omitted_ret_type,
                                 stmt->line_num));
       } else {
-        return CheckOrEnsureReturn(stmt->GetSequence().stmt, void_return,
+        return CheckOrEnsureReturn(stmt->GetSequence().stmt, omitted_ret_type,
                                    stmt->line_num);
       }
     case StatementKind::Continuation:
@@ -811,14 +824,15 @@ auto CheckOrEnsureReturn(const Statement* stmt, bool void_return, int line_num)
     case StatementKind::Break:
     case StatementKind::Continue:
     case StatementKind::VariableDefinition:
-      if (void_return) {
+      if (omitted_ret_type) {
         return Statement::MakeSequence(
             stmt->line_num, stmt,
             Statement::MakeReturn(line_num, nullptr,
                                   /*is_omitted_exp=*/true));
       } else {
         FATAL_COMPILATION_ERROR(stmt->line_num)
-            << "control-flow reaches end of non-void function without a return";
+            << "control-flow reaches end of function that provides a `->` "
+               "return type without reaching a return statement";
       }
   }
 }
@@ -827,8 +841,8 @@ auto CheckOrEnsureReturn(const Statement* stmt, bool void_return, int line_num)
 // a function.
 // TODO: Add checking to function definitions to ensure that
 //   all deduced type parameters will be deduced.
-auto TypeCheckFunDef(const FunctionDefinition* f, TypeEnv types, Env values)
-    -> struct FunctionDefinition* {
+static auto TypeCheckFunDef(const FunctionDefinition* f, TypeEnv types,
+                            Env values) -> struct FunctionDefinition* {
   // Bring the deduced parameters into scope
   for (const auto& deduced : f->deduced_parameters) {
     // auto t = InterpExp(values, deduced.type);
@@ -845,17 +859,18 @@ auto TypeCheckFunDef(const FunctionDefinition* f, TypeEnv types, Env values)
                global_arena->New<IntType>(), return_type);
     // TODO: Check that main doesn't have any parameters.
   }
-  auto res = TypeCheckStmt(f->body, param_res.types, values, return_type);
-  bool void_return = TypeEqual(return_type, &TupleValue::Empty());
-  auto body = CheckOrEnsureReturn(res.stmt, void_return, f->line_num);
+  auto res = TypeCheckStmt(f->body, param_res.types, values, return_type,
+                           f->is_omitted_return_type);
+  auto body =
+      CheckOrEnsureReturn(res.stmt, f->is_omitted_return_type, f->line_num);
   return global_arena->New<FunctionDefinition>(
       f->line_num, f->name, f->deduced_parameters, f->param_pattern,
       global_arena->New<ExpressionPattern>(ReifyType(return_type, f->line_num)),
       /*is_omitted_return_type=*/false, body);
 }
 
-auto TypeOfFunDef(TypeEnv types, Env values, const FunctionDefinition* fun_def)
-    -> const Value* {
+static auto TypeOfFunDef(TypeEnv types, Env values,
+                         const FunctionDefinition* fun_def) -> const Value* {
   // Bring the deduced parameters into scope
   for (const auto& deduced : fun_def->deduced_parameters) {
     // auto t = InterpExp(values, deduced.type);
@@ -876,8 +891,8 @@ auto TypeOfFunDef(TypeEnv types, Env values, const FunctionDefinition* fun_def)
                                          param_res.type, ret);
 }
 
-auto TypeOfStructDef(const StructDefinition* sd, TypeEnv /*types*/, Env ct_top)
-    -> const Value* {
+static auto TypeOfStructDef(const StructDefinition* sd, TypeEnv /*types*/,
+                            Env ct_top) -> const Value* {
   VarValues fields;
   VarValues methods;
   for (const Member* m : sd->members) {

executable_semantics/interpreter/typecheck.h

@@ -44,12 +44,10 @@ auto TypeCheckExp(const Expression* e, TypeEnv types, Env values)
 auto TypeCheckPattern(const Pattern* p, TypeEnv types, Env values,
                       const Value* expected) -> TCPattern;
 
-auto TypeCheckStmt(const Statement*, TypeEnv, Env, Value const*&)
+auto TypeCheckStmt(const Statement* s, TypeEnv types, Env values,
+                   const Value*& ret_type, bool is_omitted_ret_type)
     -> TCStatement;
 
-auto TypeCheckFunDef(struct FunctionDefinition*, TypeEnv)
-    -> struct FunctionDefinition*;
-
 auto MakeTypeChecked(const Declaration& decl, const TypeEnv& types,
                      const Env& values) -> const Declaration*;
 auto TopLevel(const std::list<const Declaration*>& fs) -> TypeCheckContext;

executable_semantics/syntax/parser.ypp

@@ -510,10 +510,12 @@ function_definition:
     }
 | FN identifier deduced_params maybe_empty_tuple_pattern DBLARROW expression ";"
     {
+      // The return type is not considered "omitted" because it's automatic from
+      // the expression.
       $$ = FunctionDefinition(
           yylineno, $2, $3, $4,
           global_arena->New<AutoPattern>(yylineno), true,
-          Statement::MakeReturn(yylineno, $6, false));
+          Statement::MakeReturn(yylineno, $6, true));
     }
 ;
 function_declaration:

executable_semantics/test_list.bzl

@@ -66,9 +66,13 @@ TEST_LIST = [
     "pattern_variable_fail",
     "placeholder_variable",
     "record1",
+    "return_auto",
     "return_empty_explicit",
+    "return_empty_explicit_fail",
     "return_empty_implicit1",
     "return_empty_implicit2",
+    "return_empty_implicit_fail1",
+    "return_empty_implicit_fail2",
     "star",
     "struct1",
     "struct2",

executable_semantics/testdata/global_variable2.carbon

@@ -7,7 +7,7 @@
 
 var flag: i32 = 1;
 
-fn flipFlag() -> () {
+fn flipFlag() {
   flag = 0;
 }
 

executable_semantics/testdata/return_auto.carbon

@@ -0,0 +1,11 @@
+// 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 F() -> auto {
+  return 0;
+}
+
+fn main() -> i32 {
+  return F();
+}

executable_semantics/testdata/return_auto.golden

@@ -0,0 +1,2 @@
+COMPILATION ERROR: 5: syntax error, unexpected AUTO
+EXIT CODE: 255

executable_semantics/testdata/return_empty_explicit_fail.carbon

@@ -0,0 +1,12 @@
+// 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 F() {
+  return ();
+}
+
+fn main() -> i32 {
+  F();
+  return 0;
+}

executable_semantics/testdata/return_empty_explicit_fail.golden

@@ -0,0 +1,2 @@
+COMPILATION ERROR: 6: return (); should not provide a return value, to match the function's signature.
+EXIT CODE: 255

executable_semantics/testdata/return_empty_implicit_fail1.carbon

@@ -0,0 +1,11 @@
+// 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 F() -> () {
+}
+
+fn main() -> i32 {
+  F();
+  return 0;
+}

executable_semantics/testdata/return_empty_implicit_fail1.golden

@@ -0,0 +1,2 @@
+COMPILATION ERROR: 6: control-flow reaches end of function that provides a `->` return type without reaching a return statement
+EXIT CODE: 255

executable_semantics/testdata/return_empty_implicit_fail2.carbon

@@ -0,0 +1,12 @@
+// 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 F() -> () {
+  return;
+}
+
+fn main() -> i32 {
+  F();
+  return 0;
+}

executable_semantics/testdata/return_empty_implicit_fail2.golden

@@ -0,0 +1,2 @@
+COMPILATION ERROR: 6: return; should provide a return value, to match the function's signature.
+EXIT CODE: 255