Refactor Expression accessor/mutator style (#883)

executable_semantics/ast/expression.cpp

@@ -15,6 +15,7 @@
 namespace Carbon {
 
 using llvm::cast;
+using llvm::isa;
 
 auto ExpressionFromParenContents(
     Nonnull<Arena*> arena, SourceLocation source_loc,
@@ -68,7 +69,7 @@ static void PrintFields(llvm::raw_ostream& out,
                         std::string_view separator) {
   llvm::ListSeparator sep;
   for (const auto& field : fields) {
-    out << sep << "." << field.name() << separator << *field.expression();
+    out << sep << "." << field.name() << separator << field.expression();
   }
 }
 
@@ -76,12 +77,12 @@ void Expression::Print(llvm::raw_ostream& out) const {
   switch (kind()) {
     case Expression::Kind::IndexExpression: {
       const auto& index = cast<IndexExpression>(*this);
-      out << *index.Aggregate() << "[" << *index.Offset() << "]";
+      out << index.aggregate() << "[" << index.offset() << "]";
       break;
     }
     case Expression::Kind::FieldAccessExpression: {
       const auto& access = cast<FieldAccessExpression>(*this);
-      out << *access.Aggregate() << "." << access.Field();
+      out << access.aggregate() << "." << access.field();
       break;
     }
     case Expression::Kind::TupleLiteral:
@@ -100,37 +101,43 @@ void Expression::Print(llvm::raw_ostream& out) const {
       out << "}";
       break;
     case Expression::Kind::IntLiteral:
-      out << cast<IntLiteral>(*this).Val();
+      out << cast<IntLiteral>(*this).value();
       break;
     case Expression::Kind::BoolLiteral:
-      out << (cast<BoolLiteral>(*this).Val() ? "true" : "false");
+      out << (cast<BoolLiteral>(*this).value() ? "true" : "false");
       break;
     case Expression::Kind::PrimitiveOperatorExpression: {
       out << "(";
       PrimitiveOperatorExpression op = cast<PrimitiveOperatorExpression>(*this);
-      if (op.Arguments().size() == 0) {
-        PrintOp(out, op.Op());
-      } else if (op.Arguments().size() == 1) {
-        PrintOp(out, op.Op());
-        out << " " << *op.Arguments()[0];
-      } else if (op.Arguments().size() == 2) {
-        out << *op.Arguments()[0] << " ";
-        PrintOp(out, op.Op());
-        out << " " << *op.Arguments()[1];
+      switch (op.arguments().size()) {
+        case 0:
+          PrintOp(out, op.op());
+          break;
+        case 1:
+          PrintOp(out, op.op());
+          out << " " << *op.arguments()[0];
+          break;
+        case 2:
+          out << *op.arguments()[0] << " ";
+          PrintOp(out, op.op());
+          out << " " << *op.arguments()[1];
+          break;
+        default:
+          FATAL() << "Unexpected argument count: " << op.arguments().size();
       }
       out << ")";
       break;
     }
     case Expression::Kind::IdentifierExpression:
-      out << cast<IdentifierExpression>(*this).Name();
+      out << cast<IdentifierExpression>(*this).name();
       break;
     case Expression::Kind::CallExpression: {
       const auto& call = cast<CallExpression>(*this);
-      out << *call.Function();
-      if (call.Argument()->kind() == Expression::Kind::TupleLiteral) {
-        out << *call.Argument();
+      out << call.function();
+      if (isa<TupleLiteral>(call.argument())) {
+        out << call.argument();
       } else {
-        out << "(" << *call.Argument() << ")";
+        out << "(" << call.argument() << ")";
       }
       break;
     }
@@ -142,7 +149,7 @@ void Expression::Print(llvm::raw_ostream& out) const {
       break;
     case Expression::Kind::StringLiteral:
       out << "\"";
-      out.write_escaped(cast<StringLiteral>(*this).Val());
+      out.write_escaped(cast<StringLiteral>(*this).value());
       out << "\"";
       break;
     case Expression::Kind::StringTypeLiteral:
@@ -156,13 +163,13 @@ void Expression::Print(llvm::raw_ostream& out) const {
       break;
     case Expression::Kind::FunctionTypeLiteral: {
       const auto& fn = cast<FunctionTypeLiteral>(*this);
-      out << "fn " << *fn.Parameter() << " -> " << *fn.ReturnType();
+      out << "fn " << fn.parameter() << " -> " << fn.return_type();
       break;
     }
     case Expression::Kind::IntrinsicExpression:
       out << "intrinsic_expression(";
-      switch (cast<IntrinsicExpression>(*this).Intrinsic()) {
-        case IntrinsicExpression::IntrinsicKind::Print:
+      switch (cast<IntrinsicExpression>(*this).intrinsic()) {
+        case IntrinsicExpression::Intrinsic::Print:
           out << "print";
       }
       out << ")";

executable_semantics/ast/expression.h

@@ -101,8 +101,8 @@ class FieldInitializer {
 
   auto name() const -> const std::string& { return name_; }
 
-  auto expression() const -> Nonnull<const Expression*> { return expression_; }
-  auto expression() -> Nonnull<Expression*> { return expression_; }
+  auto expression() const -> const Expression& { return *expression_; }
+  auto expression() -> Expression& { return *expression_; }
 
  private:
   // The field name. Cannot be empty.
@@ -129,16 +129,16 @@ class IdentifierExpression : public Expression {
  public:
   explicit IdentifierExpression(SourceLocation source_loc, std::string name)
       : Expression(Kind::IdentifierExpression, source_loc),
-        name(std::move(name)) {}
+        name_(std::move(name)) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::IdentifierExpression;
   }
 
-  auto Name() const -> const std::string& { return name; }
+  auto name() const -> const std::string& { return name_; }
 
  private:
-  std::string name;
+  std::string name_;
 };
 
 class FieldAccessExpression : public Expression {
@@ -147,20 +147,20 @@ class FieldAccessExpression : public Expression {
                                  Nonnull<Expression*> aggregate,
                                  std::string field)
       : Expression(Kind::FieldAccessExpression, source_loc),
-        aggregate(aggregate),
-        field(std::move(field)) {}
+        aggregate_(aggregate),
+        field_(std::move(field)) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::FieldAccessExpression;
   }
 
-  auto Aggregate() const -> Nonnull<const Expression*> { return aggregate; }
-  auto Aggregate() -> Nonnull<Expression*> { return aggregate; }
-  auto Field() const -> const std::string& { return field; }
+  auto aggregate() const -> const Expression& { return *aggregate_; }
+  auto aggregate() -> Expression& { return *aggregate_; }
+  auto field() const -> const std::string& { return field_; }
 
  private:
-  Nonnull<Expression*> aggregate;
-  std::string field;
+  Nonnull<Expression*> aggregate_;
+  std::string field_;
 };
 
 class IndexExpression : public Expression {
@@ -169,66 +169,66 @@ class IndexExpression : public Expression {
                            Nonnull<Expression*> aggregate,
                            Nonnull<Expression*> offset)
       : Expression(Kind::IndexExpression, source_loc),
-        aggregate(aggregate),
-        offset(offset) {}
+        aggregate_(aggregate),
+        offset_(offset) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::IndexExpression;
   }
 
-  auto Aggregate() const -> Nonnull<const Expression*> { return aggregate; }
-  auto Aggregate() -> Nonnull<Expression*> { return aggregate; }
-  auto Offset() const -> Nonnull<const Expression*> { return offset; }
-  auto Offset() -> Nonnull<Expression*> { return offset; }
+  auto aggregate() const -> const Expression& { return *aggregate_; }
+  auto aggregate() -> Expression& { return *aggregate_; }
+  auto offset() const -> const Expression& { return *offset_; }
+  auto offset() -> Expression& { return *offset_; }
 
  private:
-  Nonnull<Expression*> aggregate;
-  Nonnull<Expression*> offset;
+  Nonnull<Expression*> aggregate_;
+  Nonnull<Expression*> offset_;
 };
 
 class IntLiteral : public Expression {
  public:
-  explicit IntLiteral(SourceLocation source_loc, int val)
-      : Expression(Kind::IntLiteral, source_loc), val(val) {}
+  explicit IntLiteral(SourceLocation source_loc, int value)
+      : Expression(Kind::IntLiteral, source_loc), value_(value) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::IntLiteral;
   }
 
-  auto Val() const -> int { return val; }
+  auto value() const -> int { return value_; }
 
  private:
-  int val;
+  int value_;
 };
 
 class BoolLiteral : public Expression {
  public:
-  explicit BoolLiteral(SourceLocation source_loc, bool val)
-      : Expression(Kind::BoolLiteral, source_loc), val(val) {}
+  explicit BoolLiteral(SourceLocation source_loc, bool value)
+      : Expression(Kind::BoolLiteral, source_loc), value_(value) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::BoolLiteral;
   }
 
-  auto Val() const -> bool { return val; }
+  auto value() const -> bool { return value_; }
 
  private:
-  bool val;
+  bool value_;
 };
 
 class StringLiteral : public Expression {
  public:
-  explicit StringLiteral(SourceLocation source_loc, std::string val)
-      : Expression(Kind::StringLiteral, source_loc), val(std::move(val)) {}
+  explicit StringLiteral(SourceLocation source_loc, std::string value)
+      : Expression(Kind::StringLiteral, source_loc), value_(std::move(value)) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::StringLiteral;
   }
 
-  auto Val() const -> const std::string& { return val; }
+  auto value() const -> const std::string& { return value_; }
 
  private:
-  std::string val;
+  std::string value_;
 };
 
 class StringTypeLiteral : public Expression {
@@ -317,24 +317,24 @@ class PrimitiveOperatorExpression : public Expression {
       SourceLocation source_loc, Operator op,
       std::vector<Nonnull<Expression*>> arguments)
       : Expression(Kind::PrimitiveOperatorExpression, source_loc),
-        op(op),
-        arguments(std::move(arguments)) {}
+        op_(op),
+        arguments_(std::move(arguments)) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::PrimitiveOperatorExpression;
   }
 
-  auto Op() const -> Operator { return op; }
-  auto Arguments() const -> llvm::ArrayRef<Nonnull<Expression*>> {
-    return arguments;
+  auto op() const -> Operator { return op_; }
+  auto arguments() const -> llvm::ArrayRef<Nonnull<Expression*>> {
+    return arguments_;
   }
-  auto Arguments() -> llvm::MutableArrayRef<Nonnull<Expression*>> {
-    return arguments;
+  auto arguments() -> llvm::MutableArrayRef<Nonnull<Expression*>> {
+    return arguments_;
   }
 
  private:
-  Operator op;
-  std::vector<Nonnull<Expression*>> arguments;
+  Operator op_;
+  std::vector<Nonnull<Expression*>> arguments_;
 };
 
 class CallExpression : public Expression {
@@ -343,21 +343,21 @@ class CallExpression : public Expression {
                           Nonnull<Expression*> function,
                           Nonnull<Expression*> argument)
       : Expression(Kind::CallExpression, source_loc),
-        function(function),
-        argument(argument) {}
+        function_(function),
+        argument_(argument) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::CallExpression;
   }
 
-  auto Function() const -> Nonnull<const Expression*> { return function; }
-  auto Function() -> Nonnull<Expression*> { return function; }
-  auto Argument() const -> Nonnull<const Expression*> { return argument; }
-  auto Argument() -> Nonnull<Expression*> { return argument; }
+  auto function() const -> const Expression& { return *function_; }
+  auto function() -> Expression& { return *function_; }
+  auto argument() const -> const Expression& { return *argument_; }
+  auto argument() -> Expression& { return *argument_; }
 
  private:
-  Nonnull<Expression*> function;
-  Nonnull<Expression*> argument;
+  Nonnull<Expression*> function_;
+  Nonnull<Expression*> argument_;
 };
 
 class FunctionTypeLiteral : public Expression {
@@ -367,24 +367,26 @@ class FunctionTypeLiteral : public Expression {
                                Nonnull<Expression*> return_type,
                                bool is_omitted_return_type)
       : Expression(Kind::FunctionTypeLiteral, source_loc),
-        parameter(parameter),
-        return_type(return_type),
-        is_omitted_return_type(is_omitted_return_type) {}
+        parameter_(parameter),
+        return_type_(return_type),
+        is_omitted_return_type_(is_omitted_return_type) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::FunctionTypeLiteral;
   }
 
-  auto Parameter() const -> Nonnull<const Expression*> { return parameter; }
-  auto Parameter() -> Nonnull<Expression*> { return parameter; }
-  auto ReturnType() const -> Nonnull<const Expression*> { return return_type; }
-  auto ReturnType() -> Nonnull<Expression*> { return return_type; }
-  auto IsOmittedReturnType() const -> bool { return is_omitted_return_type; }
+  auto parameter() const -> const Expression& { return *parameter_; }
+  auto parameter() -> Expression& { return *parameter_; }
+  auto return_type() const -> const Expression& { return *return_type_; }
+  auto return_type() -> Expression& { return *return_type_; }
+  auto is_omitted_return_type() const -> bool {
+    return is_omitted_return_type_;
+  }
 
  private:
-  Nonnull<Expression*> parameter;
-  Nonnull<Expression*> return_type;
-  bool is_omitted_return_type;
+  Nonnull<Expression*> parameter_;
+  Nonnull<Expression*> return_type_;
+  bool is_omitted_return_type_;
 };
 
 class BoolTypeLiteral : public Expression {
@@ -429,22 +431,22 @@ class TypeTypeLiteral : public Expression {
 
 class IntrinsicExpression : public Expression {
  public:
-  enum class IntrinsicKind {
+  enum class Intrinsic {
     Print,
   };
 
-  explicit IntrinsicExpression(IntrinsicKind intrinsic)
+  explicit IntrinsicExpression(Intrinsic intrinsic)
       : Expression(Kind::IntrinsicExpression, SourceLocation("<intrinsic>", 0)),
-        intrinsic(intrinsic) {}
+        intrinsic_(intrinsic) {}
 
   static auto classof(const Expression* exp) -> bool {
     return exp->kind() == Kind::IntrinsicExpression;
   }
 
-  auto Intrinsic() const -> IntrinsicKind { return intrinsic; }
+  auto intrinsic() const -> Intrinsic { return intrinsic_; }
 
  private:
-  IntrinsicKind intrinsic;
+  Intrinsic intrinsic_;
 };
 
 }  // namespace Carbon

executable_semantics/ast/expression_test.cpp

@@ -23,7 +23,7 @@ using testing::IsEmpty;
 // `IntLiteral`
 MATCHER_P(IntFieldNamed, name, "") {
   return arg.name() == std::string(name) &&
-         arg.expression()->kind() == Expression::Kind::IntLiteral;
+         arg.expression().kind() == Expression::Kind::IntLiteral;
 }
 
 static auto FakeSourceLoc(int line_num) -> SourceLocation {

executable_semantics/ast/pattern.cpp

@@ -90,8 +90,8 @@ 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()),
+      choice_type(&RequireFieldAccess(alternative).aggregate()),
+      alternative_name(RequireFieldAccess(alternative).field()),
       arguments(arguments) {}
 
 auto ParenExpressionToParenPattern(Nonnull<Arena*> arena,