Drop support for named tuple fields (#886)

Rationale: Based on the status of #478 and #505, Carbon won't have this feature for a while, and it will be simpler not to support it on spec in the meantime.
carbon-language · Oct 15, 2021 · 097f00e · 097f00e
1 parent a7b9f6f
commit 097f00e
Show file tree

Hide file tree

Showing 22 changed files with 175 additions and 469 deletions.
diff --git a/executable_semantics/ast/expression.cpp b/executable_semantics/ast/expression.cpp
@@ -31,8 +31,7 @@ auto ExpressionFromParenContents(
 auto TupleExpressionFromParenContents(
     Nonnull<Arena*> arena, SourceLocation source_loc,
     const ParenContents<Expression>& paren_contents) -> Nonnull<Expression*> {
-  return arena->New<TupleLiteral>(
-      source_loc, paren_contents.TupleElements<FieldInitializer>(source_loc));
+  return arena->New<TupleLiteral>(source_loc, paren_contents.elements);
 }
 
 static void PrintOp(llvm::raw_ostream& out, Operator op) {
@@ -85,11 +84,16 @@ void Expression::Print(llvm::raw_ostream& out) const {
       out << access.aggregate() << "." << access.field();
       break;
     }
-    case Expression::Kind::TupleLiteral:
+    case Expression::Kind::TupleLiteral: {
       out << "(";
-      PrintFields(out, cast<TupleLiteral>(*this).fields(), " = ");
+      llvm::ListSeparator sep;
+      for (Nonnull<const Expression*> field :
+           cast<TupleLiteral>(*this).fields()) {
+        out << sep << *field;
+      }
       out << ")";
       break;
+    }
     case Expression::Kind::StructLiteral:
       out << "{";
       PrintFields(out, cast<StructLiteral>(*this).fields(), " = ");

diff --git a/executable_semantics/ast/expression.h b/executable_semantics/ast/expression.h
@@ -92,8 +92,7 @@ auto TupleExpressionFromParenContents(
     Nonnull<Arena*> arena, SourceLocation source_loc,
     const ParenContents<Expression>& paren_contents) -> Nonnull<Expression*>;
 
-// A FieldInitializer represents the initialization of a single tuple or
-// struct field.
+// A FieldInitializer represents the initialization of a single struct field.
 class FieldInitializer {
  public:
   FieldInitializer(std::string name, Nonnull<Expression*> expression)
@@ -247,19 +246,21 @@ class TupleLiteral : public Expression {
       : TupleLiteral(source_loc, {}) {}
 
   explicit TupleLiteral(SourceLocation source_loc,
-                        std::vector<FieldInitializer> fields)
+                        std::vector<Nonnull<Expression*>> fields)
       : Expression(Kind::TupleLiteral, source_loc),
         fields_(std::move(fields)) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::TupleLiteral;
   }
 
-  auto fields() const -> llvm::ArrayRef<FieldInitializer> { return fields_; }
-  auto fields() -> llvm::MutableArrayRef<FieldInitializer> { return fields_; }
+  auto fields() const -> llvm::ArrayRef<Nonnull<const Expression*>> {
+    return fields_;
+  }
+  auto fields() -> llvm::ArrayRef<Nonnull<Expression*>> { return fields_; }
 
  private:
-  std::vector<FieldInitializer> fields_;
+  std::vector<Nonnull<Expression*>> fields_;
 };
 
 // A non-empty literal value of a struct type.

diff --git a/executable_semantics/ast/expression_test.cpp b/executable_semantics/ast/expression_test.cpp
@@ -19,12 +19,8 @@ using llvm::cast;
 using testing::ElementsAre;
 using testing::IsEmpty;
 
-// Matches a FieldInitializer named `name` whose `expression` is an
-// `IntLiteral`
-MATCHER_P(IntFieldNamed, name, "") {
-  return arg.name() == std::string(name) &&
-         arg.expression().kind() == Expression::Kind::IntLiteral;
-}
+// Matches any `IntLiteral`.
+MATCHER(IntField, "") { return arg->kind() == Expression::Kind::IntLiteral; }
 
 static auto FakeSourceLoc(int line_num) -> SourceLocation {
   return SourceLocation("<test>", line_num);
@@ -63,8 +59,7 @@ TEST_F(ExpressionTest, UnaryNoCommaAsExpression) {
   // )
   // ```
   ParenContents<Expression> contents = {
-      .elements = {{.name = std::nullopt,
-                    .term = arena.New<IntLiteral>(FakeSourceLoc(2), 42)}},
+      .elements = {arena.New<IntLiteral>(FakeSourceLoc(2), 42)},
       .has_trailing_comma = false};
 
   Nonnull<const Expression*> expression =
@@ -75,76 +70,67 @@ TEST_F(ExpressionTest, UnaryNoCommaAsExpression) {
 
 TEST_F(ExpressionTest, UnaryNoCommaAsTuple) {
   ParenContents<Expression> contents = {
-      .elements = {{.name = std::nullopt,
-                    .term = arena.New<IntLiteral>(FakeSourceLoc(2), 42)}},
+      .elements = {arena.New<IntLiteral>(FakeSourceLoc(2), 42)},
       .has_trailing_comma = false};
 
   Nonnull<const Expression*> tuple =
       TupleExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->source_loc(), FakeSourceLoc(1));
   ASSERT_EQ(tuple->kind(), Expression::Kind::TupleLiteral);
-  EXPECT_THAT(cast<TupleLiteral>(*tuple).fields(),
-              ElementsAre(IntFieldNamed("0")));
+  EXPECT_THAT(cast<TupleLiteral>(*tuple).fields(), ElementsAre(IntField()));
 }
 
 TEST_F(ExpressionTest, UnaryWithCommaAsExpression) {
   ParenContents<Expression> contents = {
-      .elements = {{.name = std::nullopt,
-                    .term = arena.New<IntLiteral>(FakeSourceLoc(2), 42)}},
+      .elements = {arena.New<IntLiteral>(FakeSourceLoc(2), 42)},
       .has_trailing_comma = true};
 
   Nonnull<const Expression*> expression =
       ExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(expression->source_loc(), FakeSourceLoc(1));
   ASSERT_EQ(expression->kind(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*expression).fields(),
-              ElementsAre(IntFieldNamed("0")));
+              ElementsAre(IntField()));
 }
 
 TEST_F(ExpressionTest, UnaryWithCommaAsTuple) {
   ParenContents<Expression> contents = {
-      .elements = {{.name = std::nullopt,
-                    .term = arena.New<IntLiteral>(FakeSourceLoc(2), 42)}},
+      .elements = {arena.New<IntLiteral>(FakeSourceLoc(2), 42)},
       .has_trailing_comma = true};
 
   Nonnull<const Expression*> tuple =
       TupleExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->source_loc(), FakeSourceLoc(1));
   ASSERT_EQ(tuple->kind(), Expression::Kind::TupleLiteral);
-  EXPECT_THAT(cast<TupleLiteral>(*tuple).fields(),
-              ElementsAre(IntFieldNamed("0")));
+  EXPECT_THAT(cast<TupleLiteral>(*tuple).fields(), ElementsAre(IntField()));
 }
 
 TEST_F(ExpressionTest, BinaryAsExpression) {
   ParenContents<Expression> contents = {
-      .elements = {{.name = std::nullopt,
-                    .term = arena.New<IntLiteral>(FakeSourceLoc(2), 42)},
-                   {.name = std::nullopt,
-                    .term = arena.New<IntLiteral>(FakeSourceLoc(3), 42)}},
+      .elements = {arena.New<IntLiteral>(FakeSourceLoc(2), 42),
+                   arena.New<IntLiteral>(FakeSourceLoc(3), 42)},
       .has_trailing_comma = true};
 
   Nonnull<const Expression*> expression =
       ExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(expression->source_loc(), FakeSourceLoc(1));
   ASSERT_EQ(expression->kind(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*expression).fields(),
-              ElementsAre(IntFieldNamed("0"), IntFieldNamed("1")));
+              ElementsAre(IntField(), IntField()));
 }
 
 TEST_F(ExpressionTest, BinaryAsTuple) {
   ParenContents<Expression> contents = {
-      .elements = {{.name = std::nullopt,
-                    .term = arena.New<IntLiteral>(FakeSourceLoc(2), 42)},
-                   {.name = std::nullopt,
-                    .term = arena.New<IntLiteral>(FakeSourceLoc(3), 42)}},
+      .elements = {arena.New<IntLiteral>(FakeSourceLoc(2), 42),
+                   arena.New<IntLiteral>(FakeSourceLoc(3), 42)},
       .has_trailing_comma = true};
 
   Nonnull<const Expression*> tuple =
       TupleExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->source_loc(), FakeSourceLoc(1));
   ASSERT_EQ(tuple->kind(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*tuple).fields(),
-              ElementsAre(IntFieldNamed("0"), IntFieldNamed("1")));
+              ElementsAre(IntField(), IntField()));
 }
 
 }  // namespace

diff --git a/executable_semantics/ast/paren_contents.h b/executable_semantics/ast/paren_contents.h
@@ -26,63 +26,26 @@ namespace Carbon {
 // either `Expression` or `Pattern`.
 template <typename Term>
 struct ParenContents {
-  struct Element {
-    std::optional<std::string> name;
-    Nonnull<Term*> term;
-  };
-
-  // If this object represents a single term, with no name and no trailing
-  // comma, this method returns that term. This typically means the parentheses
-  // can be interpreted as grouping.
+  // If this object represents a single term with no trailing comma, this
+  // method returns that term. This typically means the parentheses can be
+  // interpreted as grouping.
   auto SingleTerm() const -> std::optional<Nonnull<Term*>>;
 
-  // Converts `elements` to std::vector<TupleElement>. TupleElement must
-  // have a constructor that takes a std::string and a Nonnull<Term*>.
-  //
-  // TODO: Find a way to deduce TupleElement from Term.
-  template <typename TupleElement>
-  auto TupleElements(SourceLocation source_loc) const
-      -> std::vector<TupleElement>;
-
-  std::vector<Element> elements;
+  std::vector<Nonnull<Term*>> elements;
   bool has_trailing_comma;
 };
 
 // Implementation details only below here.
 
 template <typename Term>
 auto ParenContents<Term>::SingleTerm() const -> std::optional<Nonnull<Term*>> {
-  if (elements.size() == 1 && !elements.front().name.has_value() &&
-      !has_trailing_comma) {
-    return elements.front().term;
+  if (elements.size() == 1 && !has_trailing_comma) {
+    return elements.front();
   } else {
     return std::nullopt;
   }
 }
 
-template <typename Term>
-template <typename TupleElement>
-auto ParenContents<Term>::TupleElements(SourceLocation source_loc) const
-    -> std::vector<TupleElement> {
-  std::vector<TupleElement> result;
-  int i = 0;
-  bool seen_named_member = false;
-  for (auto element : elements) {
-    if (element.name.has_value()) {
-      seen_named_member = true;
-      result.push_back(TupleElement(*element.name, element.term));
-    } else {
-      if (seen_named_member) {
-        FATAL_PROGRAM_ERROR(source_loc)
-            << "positional members must come before named members";
-      }
-      result.push_back(TupleElement(std::to_string(i), element.term));
-    }
-    ++i;
-  }
-  return result;
-}
-
 }  // namespace Carbon
 
 #endif  // EXECUTABLE_SEMANTICS_AST_PAREN_CONTENTS_H_
diff --git a/executable_semantics/ast/pattern.cpp b/executable_semantics/ast/pattern.cpp
@@ -36,8 +36,8 @@ void Pattern::Print(llvm::raw_ostream& out) const {
       const auto& tuple = cast<TuplePattern>(*this);
       out << "(";
       llvm::ListSeparator sep;
-      for (const TuplePattern::Field& field : tuple.Fields()) {
-        out << sep << field.name << " = " << *field.pattern;
+      for (Nonnull<const Pattern*> field : tuple.Fields()) {
+        out << sep << *field;
       }
       out << ")";
       break;
@@ -69,9 +69,7 @@ auto TuplePatternFromParenContents(Nonnull<Arena*> arena,
                                    SourceLocation source_loc,
                                    const ParenContents<Pattern>& paren_contents)
     -> Nonnull<TuplePattern*> {
-  return arena->New<TuplePattern>(
-      source_loc,
-      paren_contents.TupleElements<TuplePattern::Field>(source_loc));
+  return arena->New<TuplePattern>(source_loc, paren_contents.elements);
 }
 
 // Used by AlternativePattern for constructor initialization. Produces a helpful
@@ -100,9 +98,7 @@ auto ParenExpressionToParenPattern(Nonnull<Arena*> arena,
   ParenContents<Pattern> result = {
       .elements = {}, .has_trailing_comma = contents.has_trailing_comma};
   for (const auto& element : contents.elements) {
-    result.elements.push_back(
-        {.name = element.name,
-         .term = arena->New<ExpressionPattern>(element.term)});
+    result.elements.push_back(arena->New<ExpressionPattern>(element));
   }
   return result;
 }

diff --git a/executable_semantics/ast/pattern.h b/executable_semantics/ast/pattern.h
@@ -114,30 +114,20 @@ class BindingPattern : public Pattern {
 // A pattern that matches a tuple value field-wise.
 class TuplePattern : public Pattern {
  public:
-  // Represents a portion of a tuple pattern corresponding to a single field.
-  struct Field {
-    Field(std::string name, Nonnull<Pattern*> pattern)
-        : name(std::move(name)), pattern(pattern) {}
-
-    // The field name. Cannot be empty
-    std::string name;
-
-    // The pattern the field must match.
-    Nonnull<Pattern*> pattern;
-  };
-
-  TuplePattern(SourceLocation source_loc, std::vector<Field> fields)
+  TuplePattern(SourceLocation source_loc, std::vector<Nonnull<Pattern*>> fields)
       : Pattern(Kind::TuplePattern, source_loc), fields(std::move(fields)) {}
 
   static auto classof(const Pattern* pattern) -> bool {
     return pattern->kind() == Kind::TuplePattern;
   }
 
-  auto Fields() const -> llvm::ArrayRef<Field> { return fields; }
-  auto Fields() -> llvm::MutableArrayRef<Field> { return fields; }
+  auto Fields() const -> llvm::ArrayRef<Nonnull<const Pattern*>> {
+    return fields;
+  }
+  auto Fields() -> llvm::ArrayRef<Nonnull<Pattern*>> { return fields; }
 
  private:
-  std::vector<Field> fields;
+  std::vector<Nonnull<Pattern*>> fields;
 };
 
 // Converts paren_contents to a Pattern, interpreting the parentheses as