From 36b9d997b78bdad33ec4df0a04524fd3ba3a4b20 Mon Sep 17 00:00:00 2001
From: Richard Smith <richard@metafoo.co.uk>
Date: Thu, 9 Jun 2022 09:31:56 -0700
Subject: [PATCH] Disallow unexpected kinds of members in interfaces and impls.
 (#1318)

The syntax permitted in an `interface` is similar to, but different from, that permitted elsewhere. For example, associated constant declarations (`let T:! Type;`) with no initializers are valid in interfaces but nowhere else, and are a different kind of declaration from what a normal `let` declaration would produce both syntactically and semantically.

Because interfaces and impls permit only a very small subset of the complete set of kinds of declaration, it's simpler to list only the kinds that are permitted rather than to allow an arbitrary declaration and semantically disallow the invalid cases, and it's also likely to lead to better error messages as we won't try to parse unintended or meaningless things.
---
 explorer/syntax/parser.ypp                    | 29 +++++++++++++++++--
 .../testdata/impl/fail_bad_member_kind.carbon | 23 +++++++++++++++
 .../interface/fail_bad_member_kind.carbon     | 18 ++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 explorer/testdata/impl/fail_bad_member_kind.carbon
 create mode 100644 explorer/testdata/interface/fail_bad_member_kind.carbon

diff --git a/explorer/syntax/parser.ypp b/explorer/syntax/parser.ypp
index 4014e999f2e7..24c537a09949 100644
--- a/explorer/syntax/parser.ypp
+++ b/explorer/syntax/parser.ypp
@@ -108,6 +108,8 @@
 %type <Nonnull<FunctionDeclaration*>> function_declaration
 %type <Nonnull<AliasDeclaration*>> alias_declaration
 %type <std::vector<Nonnull<Declaration*>>> declaration_list
+%type <std::vector<Nonnull<Declaration*>>> interface_body
+%type <std::vector<Nonnull<Declaration*>>> impl_body
 %type <Nonnull<Statement*>> statement
 %type <Nonnull<If*>> if_statement
 %type <std::optional<Nonnull<Block*>>> optional_else
@@ -929,7 +931,7 @@ declaration:
       $$ = arena->New<VariableDeclaration>(context.source_loc(), $2, $4,
                                            ValueCategory::Let);
     }
-| INTERFACE identifier type_params LEFT_CURLY_BRACE declaration_list RIGHT_CURLY_BRACE
+| INTERFACE identifier type_params LEFT_CURLY_BRACE interface_body RIGHT_CURLY_BRACE
     {
       // TODO: Type of `Self` should be the interface being declared, not
       // `Type`.
@@ -940,7 +942,7 @@ declaration:
       $$ = arena->New<InterfaceDeclaration>(context.source_loc(), $2, $3, self,
                                             $5);
     }
-| impl_kind IMPL impl_deduced_params impl_type AS expression LEFT_CURLY_BRACE declaration_list RIGHT_CURLY_BRACE
+| impl_kind IMPL impl_deduced_params impl_type AS expression LEFT_CURLY_BRACE impl_body RIGHT_CURLY_BRACE
     {
       ErrorOr<ImplDeclaration*> impl = ImplDeclaration::Create(
           arena, context.source_loc(), $1, $4, $6, $3, $8);
@@ -974,4 +976,27 @@ declaration_list:
       $$.push_back(Nonnull<Declaration*>($2));
     }
 ;
+interface_body:
+  // Empty
+    { $$ = {}; }
+| interface_body function_declaration
+    {
+      $$ = $1;
+      $$.push_back($2);
+    }
+;
+impl_body:
+  // Empty
+    { $$ = {}; }
+| impl_body function_declaration
+    {
+      $$ = $1;
+      $$.push_back($2);
+    }
+| impl_body alias_declaration
+    {
+      $$ = $1;
+      $$.push_back($2);
+    }
+;
 %%
diff --git a/explorer/testdata/impl/fail_bad_member_kind.carbon b/explorer/testdata/impl/fail_bad_member_kind.carbon
new file mode 100644
index 000000000000..189993f2156c
--- /dev/null
+++ b/explorer/testdata/impl/fail_bad_member_kind.carbon
@@ -0,0 +1,23 @@
+// 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} %{explorer} %s 2>&1 | \
+// RUN:   %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s
+// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \
+// RUN:   %{FileCheck} --match-full-lines --allow-unused-prefixes %s
+// AUTOUPDATE: %{explorer} %s
+
+package ExplorerTest api;
+
+interface A {
+  // TODO: Use `let T:! Type;` here once we support it.
+  fn T();
+}
+
+external impl i32 as A {
+  // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_bad_member_kind.carbon:[[@LINE+1]]: syntax error, unexpected CLASS, expecting ALIAS or FN or RIGHT_CURLY_BRACE
+  class T {}
+}
+
+fn Main() -> i32 { return 0; }
diff --git a/explorer/testdata/interface/fail_bad_member_kind.carbon b/explorer/testdata/interface/fail_bad_member_kind.carbon
new file mode 100644
index 000000000000..012bb230975d
--- /dev/null
+++ b/explorer/testdata/interface/fail_bad_member_kind.carbon
@@ -0,0 +1,18 @@
+// 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} %{explorer} %s 2>&1 | \
+// RUN:   %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s
+// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \
+// RUN:   %{FileCheck} --match-full-lines --allow-unused-prefixes %s
+// AUTOUPDATE: %{explorer} %s
+
+package ExplorerTest api;
+
+interface Bad {
+  // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/interface/fail_bad_member_kind.carbon:[[@LINE+1]]: syntax error, unexpected VAR, expecting FN or RIGHT_CURLY_BRACE
+  var V: i32;
+}
+
+fn Main() -> i32 { return 0; }