Refactor Pattern and Member accessors. (#889)

executable_semantics/ast/member.cpp

@@ -15,7 +15,7 @@ void Member::Print(llvm::raw_ostream& out) const {
   switch (kind()) {
     case Kind::FieldMember:
       const auto& field = cast<FieldMember>(*this);
-      out << "var " << *field.Binding() << ";\n";
+      out << "var " << field.binding() << ";\n";
       break;
   }
 }

executable_semantics/ast/member.h

@@ -54,19 +54,19 @@ class Member {
 class FieldMember : public Member {
  public:
   FieldMember(SourceLocation source_loc, Nonnull<const BindingPattern*> binding)
-      : Member(Kind::FieldMember, source_loc), binding(binding) {}
+      : Member(Kind::FieldMember, source_loc), binding_(binding) {}
 
   static auto classof(const Member* member) -> bool {
     return member->kind() == Kind::FieldMember;
   }
 
-  auto Binding() const -> Nonnull<const BindingPattern*> { return binding; }
+  auto binding() const -> const BindingPattern& { return *binding_; }
 
  private:
   // TODO: split this into a non-optional name and a type, initialized by
   // a constructor that takes a BindingPattern and handles errors like a
   // missing name.
-  Nonnull<const BindingPattern*> binding;
+  Nonnull<const BindingPattern*> binding_;
 };
 
 }  // namespace Carbon

executable_semantics/ast/pattern.cpp

@@ -24,19 +24,19 @@ void Pattern::Print(llvm::raw_ostream& out) const {
       break;
     case Kind::BindingPattern: {
       const auto& binding = cast<BindingPattern>(*this);
-      if (binding.Name().has_value()) {
-        out << *binding.Name();
+      if (binding.name().has_value()) {
+        out << *binding.name();
       } else {
         out << "_";
       }
-      out << ": " << *binding.Type();
+      out << ": " << binding.type();
       break;
     }
     case Kind::TuplePattern: {
       const auto& tuple = cast<TuplePattern>(*this);
       out << "(";
       llvm::ListSeparator sep;
-      for (Nonnull<const Pattern*> field : tuple.Fields()) {
+      for (Nonnull<const Pattern*> field : tuple.fields()) {
         out << sep << *field;
       }
       out << ")";
@@ -44,12 +44,12 @@ void Pattern::Print(llvm::raw_ostream& out) const {
     }
     case Kind::AlternativePattern: {
       const auto& alternative = cast<AlternativePattern>(*this);
-      out << *alternative.ChoiceType() << "." << alternative.AlternativeName()
-          << *alternative.Arguments();
+      out << alternative.choice_type() << "." << alternative.alternative_name()
+          << alternative.arguments();
       break;
     }
     case Kind::ExpressionPattern:
-      out << *cast<ExpressionPattern>(*this).Expression();
+      out << cast<ExpressionPattern>(*this).expression();
       break;
   }
 }
@@ -88,9 +88,9 @@ AlternativePattern::AlternativePattern(SourceLocation source_loc,
                                        Nonnull<Expression*> alternative,
                                        Nonnull<TuplePattern*> arguments)
     : Pattern(Kind::AlternativePattern, source_loc),
-      choice_type(&RequireFieldAccess(alternative).aggregate()),
-      alternative_name(RequireFieldAccess(alternative).field()),
-      arguments(arguments) {}
+      choice_type_(&RequireFieldAccess(alternative).aggregate()),
+      alternative_name_(RequireFieldAccess(alternative).field()),
+      arguments_(arguments) {}
 
 auto ParenExpressionToParenPattern(Nonnull<Arena*> arena,
                                    const ParenContents<Expression>& contents)

executable_semantics/ast/pattern.h

@@ -92,42 +92,42 @@ class BindingPattern : public Pattern {
   BindingPattern(SourceLocation source_loc, std::optional<std::string> name,
                  Nonnull<Pattern*> type)
       : Pattern(Kind::BindingPattern, source_loc),
-        name(std::move(name)),
-        type(type) {}
+        name_(std::move(name)),
+        type_(type) {}
 
   static auto classof(const Pattern* pattern) -> bool {
     return pattern->kind() == Kind::BindingPattern;
   }
 
   // The name this pattern binds, if any.
-  auto Name() const -> const std::optional<std::string>& { return name; }
+  auto name() const -> const std::optional<std::string>& { return name_; }
 
   // The pattern specifying the type of values that this pattern matches.
-  auto Type() const -> Nonnull<const Pattern*> { return type; }
-  auto Type() -> Nonnull<Pattern*> { return type; }
+  auto type() const -> const Pattern& { return *type_; }
+  auto type() -> Pattern& { return *type_; }
 
  private:
-  std::optional<std::string> name;
-  Nonnull<Pattern*> type;
+  std::optional<std::string> name_;
+  Nonnull<Pattern*> type_;
 };
 
 // A pattern that matches a tuple value field-wise.
 class TuplePattern : public Pattern {
  public:
   TuplePattern(SourceLocation source_loc, std::vector<Nonnull<Pattern*>> fields)
-      : Pattern(Kind::TuplePattern, source_loc), fields(std::move(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<Nonnull<const Pattern*>> {
-    return fields;
+  auto fields() const -> llvm::ArrayRef<Nonnull<const Pattern*>> {
+    return fields_;
   }
-  auto Fields() -> llvm::ArrayRef<Nonnull<Pattern*>> { return fields; }
+  auto fields() -> llvm::ArrayRef<Nonnull<Pattern*>> { return fields_; }
 
  private:
-  std::vector<Nonnull<Pattern*>> fields;
+  std::vector<Nonnull<Pattern*>> fields_;
 };
 
 // Converts paren_contents to a Pattern, interpreting the parentheses as
@@ -161,9 +161,9 @@ class AlternativePattern : public Pattern {
                      std::string alternative_name,
                      Nonnull<TuplePattern*> arguments)
       : Pattern(Kind::AlternativePattern, source_loc),
-        choice_type(choice_type),
-        alternative_name(std::move(alternative_name)),
-        arguments(arguments) {}
+        choice_type_(choice_type),
+        alternative_name_(std::move(alternative_name)),
+        arguments_(arguments) {}
 
   // Constructs an AlternativePattern that matches the alternative specified
   // by `alternative`, if its arguments match `arguments`.
@@ -175,18 +175,18 @@ class AlternativePattern : public Pattern {
     return pattern->kind() == Kind::AlternativePattern;
   }
 
-  auto ChoiceType() const -> Nonnull<const Expression*> { return choice_type; }
-  auto ChoiceType() -> Nonnull<Expression*> { return choice_type; }
-  auto AlternativeName() const -> const std::string& {
-    return alternative_name;
+  auto choice_type() const -> const Expression& { return *choice_type_; }
+  auto choice_type() -> Expression& { return *choice_type_; }
+  auto alternative_name() const -> const std::string& {
+    return alternative_name_;
   }
-  auto Arguments() const -> Nonnull<const TuplePattern*> { return arguments; }
-  auto Arguments() -> Nonnull<TuplePattern*> { return arguments; }
+  auto arguments() const -> const TuplePattern& { return *arguments_; }
+  auto arguments() -> TuplePattern& { return *arguments_; }
 
  private:
-  Nonnull<Expression*> choice_type;
-  std::string alternative_name;
-  Nonnull<TuplePattern*> arguments;
+  Nonnull<Expression*> choice_type_;
+  std::string alternative_name_;
+  Nonnull<TuplePattern*> arguments_;
 };
 
 // A pattern that matches a value if it is equal to the value of a given
@@ -195,17 +195,17 @@ class ExpressionPattern : public Pattern {
  public:
   ExpressionPattern(Nonnull<Expression*> expression)
       : Pattern(Kind::ExpressionPattern, expression->source_loc()),
-        expression(expression) {}
+        expression_(expression) {}
 
   static auto classof(const Pattern* pattern) -> bool {
     return pattern->kind() == Kind::ExpressionPattern;
   }
 
-  auto Expression() const -> Nonnull<const Expression*> { return expression; }
-  auto Expression() -> Nonnull<Carbon::Expression*> { return expression; }
+  auto expression() const -> const Expression& { return *expression_; }
+  auto expression() -> Expression& { return *expression_; }
 
  private:
-  Nonnull<Carbon::Expression*> expression;
+  Nonnull<Expression*> expression_;
 };
 
 }  // namespace Carbon

executable_semantics/ast/pattern_test.cpp

@@ -38,7 +38,7 @@ TEST_F(PatternTest, EmptyAsPattern) {
       PatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(pattern->source_loc(), FakeSourceLoc(1));
   ASSERT_TRUE(isa<TuplePattern>(*pattern));
-  EXPECT_THAT(cast<TuplePattern>(*pattern).Fields(), IsEmpty());
+  EXPECT_THAT(cast<TuplePattern>(*pattern).fields(), IsEmpty());
 }
 
 TEST_F(PatternTest, EmptyAsTuplePattern) {
@@ -47,7 +47,7 @@ TEST_F(PatternTest, EmptyAsTuplePattern) {
   Nonnull<const TuplePattern*> tuple =
       TuplePatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->source_loc(), FakeSourceLoc(1));
-  EXPECT_THAT(tuple->Fields(), IsEmpty());
+  EXPECT_THAT(tuple->fields(), IsEmpty());
 }
 
 TEST_F(PatternTest, UnaryNoCommaAsPattern) {
@@ -75,7 +75,7 @@ TEST_F(PatternTest, UnaryNoCommaAsTuplePattern) {
   Nonnull<const TuplePattern*> tuple =
       TuplePatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->source_loc(), FakeSourceLoc(1));
-  EXPECT_THAT(tuple->Fields(), ElementsAre(AutoField()));
+  EXPECT_THAT(tuple->fields(), ElementsAre(AutoField()));
 }
 
 TEST_F(PatternTest, UnaryWithCommaAsPattern) {
@@ -87,7 +87,7 @@ TEST_F(PatternTest, UnaryWithCommaAsPattern) {
       PatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(pattern->source_loc(), FakeSourceLoc(1));
   ASSERT_TRUE(isa<TuplePattern>(*pattern));
-  EXPECT_THAT(cast<TuplePattern>(*pattern).Fields(), ElementsAre(AutoField()));
+  EXPECT_THAT(cast<TuplePattern>(*pattern).fields(), ElementsAre(AutoField()));
 }
 
 TEST_F(PatternTest, UnaryWithCommaAsTuplePattern) {
@@ -98,7 +98,7 @@ TEST_F(PatternTest, UnaryWithCommaAsTuplePattern) {
   Nonnull<const TuplePattern*> tuple =
       TuplePatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->source_loc(), FakeSourceLoc(1));
-  EXPECT_THAT(tuple->Fields(), ElementsAre(AutoField()));
+  EXPECT_THAT(tuple->fields(), ElementsAre(AutoField()));
 }
 
 TEST_F(PatternTest, BinaryAsPattern) {
@@ -111,7 +111,7 @@ TEST_F(PatternTest, BinaryAsPattern) {
       PatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(pattern->source_loc(), FakeSourceLoc(1));
   ASSERT_TRUE(isa<TuplePattern>(*pattern));
-  EXPECT_THAT(cast<TuplePattern>(*pattern).Fields(),
+  EXPECT_THAT(cast<TuplePattern>(*pattern).fields(),
               ElementsAre(AutoField(), AutoField()));
 }
 
@@ -124,7 +124,7 @@ TEST_F(PatternTest, BinaryAsTuplePattern) {
   Nonnull<const TuplePattern*> tuple =
       TuplePatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->source_loc(), FakeSourceLoc(1));
-  EXPECT_THAT(tuple->Fields(), ElementsAre(AutoField(), AutoField()));
+  EXPECT_THAT(tuple->fields(), ElementsAre(AutoField(), AutoField()));
 }
 
 }  // namespace

executable_semantics/interpreter/interpreter.cpp

@@ -131,12 +131,11 @@ void Interpreter::InitEnv(const Declaration& d, Env* env) {
       for (Nonnull<const Member*> m : class_def.members()) {
         switch (m->kind()) {
           case Member::Kind::FieldMember: {
-            Nonnull<const BindingPattern*> binding =
-                cast<FieldMember>(*m).Binding();
-            Nonnull<const Expression*> type_expression =
-                cast<ExpressionPattern>(*binding->Type()).Expression();
-            auto type = InterpExp(Env(arena), type_expression);
-            fields.push_back(make_pair(*binding->Name(), type));
+            const BindingPattern& binding = cast<FieldMember>(*m).binding();
+            const Expression& type_expression =
+                cast<ExpressionPattern>(binding.type()).expression();
+            auto type = InterpExp(Env(arena), &type_expression);
+            fields.push_back(make_pair(*binding.name(), type));
             break;
           }
         }
@@ -167,7 +166,7 @@ void Interpreter::InitEnv(const Declaration& d, Env* env) {
       // result of evaluating the initializer.
       auto v = InterpExp(*env, &var.initializer());
       Address a = heap.AllocateValue(v);
-      env->Set(*var.binding().Name(), a);
+      env->Set(*var.binding().name(), a);
       break;
     }
   }
@@ -674,20 +673,20 @@ auto Interpreter::StepPattern() -> Transition {
     case Pattern::Kind::BindingPattern: {
       const auto& binding = cast<BindingPattern>(*pattern);
       if (act->pos() == 0) {
-        return Spawn{arena->New<PatternAction>(binding.Type())};
+        return Spawn{arena->New<PatternAction>(&binding.type())};
       } else {
-        return Done{arena->New<BindingPlaceholderValue>(binding.Name(),
+        return Done{arena->New<BindingPlaceholderValue>(binding.name(),
                                                         act->results()[0])};
       }
     }
     case Pattern::Kind::TuplePattern: {
       const auto& tuple = cast<TuplePattern>(*pattern);
-      if (act->pos() < static_cast<int>(tuple.Fields().size())) {
+      if (act->pos() < static_cast<int>(tuple.fields().size())) {
         //    { { vk :: (f1=v1,..., fk=[],fk+1=ek+1,...) :: C, E, F} :: S,
         //    H}
         // -> { { ek+1 :: (f1=v1,..., fk=vk, fk+1=[],...) :: C, E, F} :: S,
         // H}
-        return Spawn{arena->New<PatternAction>(tuple.Fields()[act->pos()])};
+        return Spawn{arena->New<PatternAction>(tuple.fields()[act->pos()])};
       } else {
         return Done{arena->New<TupleValue>(act->results())};
       }
@@ -695,20 +694,20 @@ auto Interpreter::StepPattern() -> Transition {
     case Pattern::Kind::AlternativePattern: {
       const auto& alternative = cast<AlternativePattern>(*pattern);
       if (act->pos() == 0) {
-        return Spawn{arena->New<ExpressionAction>(alternative.ChoiceType())};
+        return Spawn{arena->New<ExpressionAction>(&alternative.choice_type())};
       } else if (act->pos() == 1) {
-        return Spawn{arena->New<PatternAction>(alternative.Arguments())};
+        return Spawn{arena->New<PatternAction>(&alternative.arguments())};
       } else {
         CHECK(act->pos() == 2);
         const auto& choice_type = cast<ChoiceType>(*act->results()[0]);
-        return Done{arena->New<AlternativeValue>(alternative.AlternativeName(),
+        return Done{arena->New<AlternativeValue>(alternative.alternative_name(),
                                                  choice_type.Name(),
                                                  act->results()[1])};
       }
     }
     case Pattern::Kind::ExpressionPattern:
       return Delegate{arena->New<ExpressionAction>(
-          cast<ExpressionPattern>(*pattern).Expression())};
+          &cast<ExpressionPattern>(*pattern).expression())};
   }
 }
 

executable_semantics/interpreter/type_checker.cpp

@@ -726,17 +726,17 @@ auto TypeChecker::TypeCheckPattern(
     }
     case Pattern::Kind::BindingPattern: {
       auto& binding = cast<BindingPattern>(*p);
-      TypeCheckPattern(binding.Type(), types, values, std::nullopt);
+      TypeCheckPattern(&binding.type(), types, values, std::nullopt);
       Nonnull<const Value*> type =
-          interpreter.InterpPattern(values, binding.Type());
+          interpreter.InterpPattern(values, &binding.type());
       if (expected) {
         if (IsConcreteType(type)) {
           ExpectType(p->source_loc(), "name binding", type, *expected);
         } else {
           std::optional<Env> values = interpreter.PatternMatch(
-              type, *expected, binding.Type()->source_loc());
+              type, *expected, binding.type().source_loc());
           if (values == std::nullopt) {
-            FATAL_COMPILATION_ERROR(binding.Type()->source_loc())
+            FATAL_COMPILATION_ERROR(binding.type().source_loc())
                 << "Type pattern '" << *type << "' does not match actual type '"
                 << **expected << "'";
           }
@@ -746,8 +746,8 @@ auto TypeChecker::TypeCheckPattern(
         }
       }
       ExpectIsConcreteType(binding.source_loc(), type);
-      if (binding.Name().has_value()) {
-        types.Set(*binding.Name(), type);
+      if (binding.name().has_value()) {
+        types.Set(*binding.name(), type);
       }
       SetStaticType(&binding, type);
       return TCResult(types);
@@ -759,13 +759,13 @@ auto TypeChecker::TypeCheckPattern(
       if (expected && (*expected)->kind() != Value::Kind::TupleValue) {
         FATAL_COMPILATION_ERROR(p->source_loc()) << "didn't expect a tuple";
       }
-      if (expected && tuple.Fields().size() !=
+      if (expected && tuple.fields().size() !=
                           cast<TupleValue>(**expected).Elements().size()) {
         FATAL_COMPILATION_ERROR(tuple.source_loc())
             << "tuples of different length";
       }
-      for (size_t i = 0; i < tuple.Fields().size(); ++i) {
-        Nonnull<Pattern*> field = tuple.Fields()[i];
+      for (size_t i = 0; i < tuple.fields().size(); ++i) {
+        Nonnull<Pattern*> field = tuple.fields()[i];
         std::optional<Nonnull<const Value*>> expected_field_type;
         if (expected) {
           expected_field_type = cast<TupleValue>(**expected).Elements()[i];
@@ -781,7 +781,7 @@ auto TypeChecker::TypeCheckPattern(
     case Pattern::Kind::AlternativePattern: {
       auto& alternative = cast<AlternativePattern>(*p);
       Nonnull<const Value*> choice_type =
-          interpreter.InterpExp(values, alternative.ChoiceType());
+          interpreter.InterpExp(values, &alternative.choice_type());
       if (choice_type->kind() != Value::Kind::ChoiceType) {
         FATAL_COMPILATION_ERROR(alternative.source_loc())
             << "alternative pattern does not name a choice type.";
@@ -791,22 +791,22 @@ auto TypeChecker::TypeCheckPattern(
                         *expected, choice_type);
       }
       std::optional<Nonnull<const Value*>> parameter_types =
-          FindInVarValues(alternative.AlternativeName(),
+          FindInVarValues(alternative.alternative_name(),
                           cast<ChoiceType>(*choice_type).Alternatives());
       if (parameter_types == std::nullopt) {
         FATAL_COMPILATION_ERROR(alternative.source_loc())
-            << "'" << alternative.AlternativeName()
+            << "'" << alternative.alternative_name()
             << "' is not an alternative of " << *choice_type;
       }
-      TCResult arg_results = TypeCheckPattern(alternative.Arguments(), types,
+      TCResult arg_results = TypeCheckPattern(&alternative.arguments(), types,
                                               values, *parameter_types);
       SetStaticType(&alternative, choice_type);
       return TCResult(arg_results.types);
     }
     case Pattern::Kind::ExpressionPattern: {
-      const auto& expression = cast<ExpressionPattern>(*p).Expression();
-      TCResult result = TypeCheckExp(expression, types, values);
-      SetStaticType(p, &expression->static_type());
+      auto& expression = cast<ExpressionPattern>(*p).expression();
+      TCResult result = TypeCheckExp(&expression, types, values);
+      SetStaticType(p, &expression.static_type());
       return TCResult(result.types);
     }
   }
@@ -1097,19 +1097,18 @@ auto TypeChecker::TypeOfClassDef(const ClassDefinition* sd, TypeEnv /*types*/,
   for (Nonnull<const Member*> m : sd->members()) {
     switch (m->kind()) {
       case Member::Kind::FieldMember: {
-        Nonnull<const BindingPattern*> binding =
-            cast<FieldMember>(*m).Binding();
-        if (!binding->Name().has_value()) {
-          FATAL_COMPILATION_ERROR(binding->source_loc())
+        const BindingPattern& binding = cast<FieldMember>(*m).binding();
+        if (!binding.name().has_value()) {
+          FATAL_COMPILATION_ERROR(binding.source_loc())
               << "Struct members must have names";
         }
-        const auto* binding_type = dyn_cast<ExpressionPattern>(binding->Type());
+        const auto* binding_type = dyn_cast<ExpressionPattern>(&binding.type());
         if (binding_type == nullptr) {
-          FATAL_COMPILATION_ERROR(binding->source_loc())
+          FATAL_COMPILATION_ERROR(binding.source_loc())
               << "Struct members must have explicit types";
         }
-        auto type = interpreter.InterpExp(ct_top, binding_type->Expression());
-        fields.push_back(std::make_pair(*binding->Name(), type));
+        auto type = interpreter.InterpExp(ct_top, &binding_type->expression());
+        fields.push_back(std::make_pair(*binding.name(), type));
         break;
       }
     }
@@ -1128,11 +1127,11 @@ static auto GetName(const Declaration& d) -> const std::string& {
       return cast<ChoiceDeclaration>(d).name();
     case Declaration::Kind::VariableDeclaration: {
       const BindingPattern& binding = cast<VariableDeclaration>(d).binding();
-      if (!binding.Name().has_value()) {
+      if (!binding.name().has_value()) {
         FATAL_COMPILATION_ERROR(binding.source_loc())
             << "Top-level variable declarations must have names";
       }
-      return *binding.Name();
+      return *binding.name();
     }
   }
 }
@@ -1159,14 +1158,14 @@ void TypeChecker::TypeCheck(Nonnull<Declaration*> d, const TypeEnv& types,
       // declaration with annotated types.
       TypeCheckExp(&var.initializer(), types, values);
       const auto* binding_type =
-          dyn_cast<ExpressionPattern>(var.binding().Type());
+          dyn_cast<ExpressionPattern>(&var.binding().type());
       if (binding_type == nullptr) {
         // TODO: consider adding support for `auto`
         FATAL_COMPILATION_ERROR(var.source_loc())
             << "Type of a top-level variable must be an expression.";
       }
       Nonnull<const Value*> declared_type =
-          interpreter.InterpExp(values, binding_type->Expression());
+          interpreter.InterpExp(values, &binding_type->expression());
       ExpectType(var.source_loc(), "initializer of variable", declared_type,
                  &var.initializer().static_type());
       return;
@@ -1212,11 +1211,11 @@ void TypeChecker::TopLevel(Nonnull<Declaration*> d, TypeCheckContext* tops) {
       auto& var = cast<VariableDeclaration>(*d);
       // Associate the variable name with it's declared type in the
       // compile-time symbol table.
-      Nonnull<Expression*> type =
-          cast<ExpressionPattern>(*var.binding().Type()).Expression();
+      Expression& type =
+          cast<ExpressionPattern>(var.binding().type()).expression();
       Nonnull<const Value*> declared_type =
-          interpreter.InterpExp(tops->values, type);
-      tops->types.Set(*var.binding().Name(), declared_type);
+          interpreter.InterpExp(tops->values, &type);
+      tops->types.Set(*var.binding().name(), declared_type);
       break;
     }
   }