Expect parentheses after an alternative only if they were present in its declaration (#2605)

The design of choice types expects the declaration of an alternative to match the usage: if an alternative is declared as `None`, then it should be used as `None` not `None()`, and if it is declared as `None()` then it should be used as `None()` not `None`. Update explorer to match.

Also clean up the handling of choice types and alternative values a little in general, by moving away from identifying choice types and alternatives as strings and towards identifying them symbolically.

Closes #2422

common/fuzzing/proto_to_carbon.cpp

@@ -853,7 +853,9 @@ static auto DeclarationToCarbon(const Fuzzing::Declaration& declaration,
       for (const auto& alternative : choice.alternatives()) {
         out << sep;
         IdentifierToCarbon(alternative.name(), out);
-        TupleLiteralExpressionToCarbon(alternative.signature(), out);
+        if (alternative.has_signature()) {
+          TupleLiteralExpressionToCarbon(alternative.signature(), out);
+        }
       }
       out << "}";
       break;

explorer/ast/declaration.cpp

@@ -429,11 +429,24 @@ auto ImplDeclaration::Create(Nonnull<Arena*> arena, SourceLocation source_loc,
 }
 
 void AlternativeSignature::Print(llvm::raw_ostream& out) const {
-  out << "alt " << name() << " " << signature();
+  out << "alt " << name();
+  if (auto sig = signature()) {
+    out << **signature();
+  }
 }
 
 void AlternativeSignature::PrintID(llvm::raw_ostream& out) const {
   out << name();
 }
 
+auto ChoiceDeclaration::FindAlternative(std::string_view name) const
+    -> std::optional<const AlternativeSignature*> {
+  for (auto* alt : alternatives()) {
+    if (alt->name() == name) {
+      return alt;
+    }
+  }
+  return std::nullopt;
+}
+
 }  // namespace Carbon

explorer/ast/declaration.h

@@ -468,7 +468,7 @@ class MixDeclaration : public Declaration {
 class AlternativeSignature : public AstNode {
  public:
   AlternativeSignature(SourceLocation source_loc, std::string name,
-                       Nonnull<TupleLiteral*> signature)
+                       std::optional<Nonnull<TupleLiteral*>> signature)
       : AstNode(AstNodeKind::AlternativeSignature, source_loc),
         name_(std::move(name)),
         signature_(signature) {}
@@ -481,12 +481,29 @@ class AlternativeSignature : public AstNode {
   }
 
   auto name() const -> const std::string& { return name_; }
-  auto signature() const -> const TupleLiteral& { return *signature_; }
-  auto signature() -> TupleLiteral& { return *signature_; }
+  auto signature() const -> std::optional<Nonnull<const TupleLiteral*>> {
+    return signature_;
+  }
+  auto signature() -> std::optional<Nonnull<TupleLiteral*>> {
+    return signature_;
+  }
+
+  // The static type signature, if any. Cannot be called before type checking.
+  auto static_type() const -> std::optional<Nonnull<const Value*>> {
+    return static_type_;
+  }
+
+  // Sets the static type of the declared entity. Can only be called once,
+  // during typechecking.
+  void set_static_type(Nonnull<const Value*> type) {
+    CARBON_CHECK(!static_type_.has_value());
+    static_type_ = type;
+  }
 
  private:
   std::string name_;
-  Nonnull<TupleLiteral*> signature_;
+  std::optional<Nonnull<TupleLiteral*>> signature_;
+  std::optional<Nonnull<const Value*>> static_type_;
 };
 
 class ChoiceDeclaration : public Declaration {
@@ -522,18 +539,15 @@ class ChoiceDeclaration : public Declaration {
     return alternatives_;
   }
 
-  void set_members(const std::vector<NamedValue>& members) {
-    members_ = members;
-  }
-  auto members() const -> std::vector<NamedValue> { return members_; }
-
   auto value_category() const -> ValueCategory { return ValueCategory::Let; }
 
+  auto FindAlternative(std::string_view name) const
+      -> std::optional<const AlternativeSignature*>;
+
  private:
   DeclaredName name_;
   std::optional<Nonnull<TuplePattern*>> type_params_;
   std::vector<Nonnull<AlternativeSignature*>> alternatives_;
-  std::vector<NamedValue> members_;
 };
 
 // Global variable definition implements the Declaration concept.

explorer/data/prelude.carbon

@@ -181,15 +181,15 @@ impl i32 as CompareWith(Self) {
   fn Compare[self: Self](other: Self) -> Ordering {
     var comp: i32 = __intrinsic_int_compare(self, other);
     if (comp == -1) {
-      return Ordering.Less();
+      return Ordering.Less;
     }
     if (comp == 0) {
-      return Ordering.Equivalent();
+      return Ordering.Equivalent;
     }
     if (comp == 1) {
-      return Ordering.Greater();
+      return Ordering.Greater;
     }
-    return Ordering.Incomparable();
+    return Ordering.Incomparable;
 
   }
 }
@@ -198,15 +198,15 @@ impl String as CompareWith(Self) {
   fn Compare[self: Self](other: Self) -> Ordering {
     var comp: i32 = __intrinsic_str_compare(self, other);
     if (comp == -1) {
-      return Ordering.Less();
+      return Ordering.Less;
     }
     if (comp == 0) {
-      return Ordering.Equivalent();
+      return Ordering.Equivalent;
     }
     if (comp == 1) {
-      return Ordering.Greater();
+      return Ordering.Greater;
     }
-    return Ordering.Incomparable();
+    return Ordering.Incomparable;
   }
 }
 
@@ -230,7 +230,7 @@ impl i32 as LessWith(Self) {
   fn Less[self: Self](other: Self) -> bool {
     var comp: Ordering = self.(CompareWith(i32).Compare)(other);
     match (comp) {
-      case Ordering.Less() => {
+      case Ordering.Less => {
         return true;
       }
     }
@@ -242,7 +242,7 @@ impl String as LessWith(Self) {
   fn Less[self: Self](other: Self) -> bool {
     var comp: Ordering =  self.(CompareWith(String).Compare)(other);
     match(comp){
-      case Ordering.Less() => {
+      case Ordering.Less => {
         return true;
       }
     }
@@ -254,10 +254,10 @@ impl i32 as LessEqWith(Self) {
   fn LessEq[self: Self](other: Self) -> bool {
     var comp: Ordering =  self.(CompareWith(i32).Compare)(other);
     match(comp){
-      case Ordering.Less() => {
+      case Ordering.Less => {
         return true;
       }
-      case Ordering.Equivalent() => {
+      case Ordering.Equivalent => {
         return true;
       }
     }
@@ -269,10 +269,10 @@ impl String as LessEqWith(Self) {
   fn LessEq[self: Self](other: Self) -> bool {
     var comp: Ordering =  self.(CompareWith(String).Compare)(other);
     match(comp){
-      case Ordering.Less() => {
+      case Ordering.Less => {
         return true;
       }
-      case Ordering.Equivalent() => {
+      case Ordering.Equivalent => {
         return true;
       }
     }
@@ -284,7 +284,7 @@ impl i32 as GreaterWith(Self) {
   fn Greater[self: Self](other: Self) -> bool {
     var comp: Ordering =  self.(CompareWith(i32).Compare)(other);
     match(comp){
-      case Ordering.Greater() => {
+      case Ordering.Greater => {
         return true;
       }
     }
@@ -296,7 +296,7 @@ impl String as GreaterWith(Self) {
   fn Greater[self: Self](other: Self) -> bool {
     var comp: Ordering =  self.(CompareWith(String).Compare)(other);
     match(comp){
-      case Ordering.Greater() => {
+      case Ordering.Greater => {
         return true;
       }
     }
@@ -308,10 +308,10 @@ impl i32 as GreaterEqWith(Self) {
   fn GreaterEq[self: Self](other: Self) -> bool {
     var comp: Ordering =  self.(CompareWith(i32).Compare)(other);
     match(comp){
-      case Ordering.Greater() => {
+      case Ordering.Greater => {
         return true;
       }
-      case Ordering.Equivalent() => {
+      case Ordering.Equivalent => {
         return true;
       }
     }
@@ -323,10 +323,10 @@ impl String as GreaterEqWith(Self) {
   fn GreaterEq[self: Self](other: Self) -> bool {
     var comp: Ordering =  self.(CompareWith(String).Compare)(other);
     match(comp){
-      case Ordering.Greater() => {
+      case Ordering.Greater => {
         return true;
       }
-      case Ordering.Equivalent() => {
+      case Ordering.Equivalent => {
         return true;
       }
     }

explorer/fuzzing/ast_to_proto.cpp

@@ -750,8 +750,10 @@ static auto DeclarationToProto(const Declaration& declaration)
            choice.alternatives()) {
         auto* alternative_proto = choice_proto->add_alternatives();
         alternative_proto->set_name(alternative->name());
-        *alternative_proto->mutable_signature() =
-            TupleLiteralExpressionToProto(alternative->signature());
+        if (auto sig = alternative->signature()) {
+          *alternative_proto->mutable_signature() =
+              TupleLiteralExpressionToProto(**sig);
+        }
       }
       break;
     }

explorer/interpreter/interpreter.cpp

@@ -350,11 +350,15 @@ auto PatternMatch(Nonnull<const Value*> p, Nonnull<const Value*> v,
         case Value::Kind::AlternativeValue: {
           const auto& p_alt = cast<AlternativeValue>(*p);
           const auto& v_alt = cast<AlternativeValue>(*v);
-          if (p_alt.choice_name() != v_alt.choice_name() ||
-              p_alt.alt_name() != v_alt.alt_name()) {
+          if (&p_alt.alternative() != &v_alt.alternative()) {
             return false;
           }
-          return PatternMatch(&p_alt.argument(), &v_alt.argument(), source_loc,
+          CARBON_CHECK(p_alt.argument().has_value() ==
+                       v_alt.argument().has_value());
+          if (!p_alt.argument().has_value()) {
+            return true;
+          }
+          return PatternMatch(*p_alt.argument(), *v_alt.argument(), source_loc,
                               bindings, generic_args, trace_stream, arena);
         }
         default:
@@ -960,7 +964,7 @@ auto Interpreter::CallFunction(const CallExpression& call,
     case Value::Kind::AlternativeConstructorValue: {
       const auto& alt = cast<AlternativeConstructorValue>(*fun);
       return todo_.FinishAction(arena_->New<AlternativeValue>(
-          alt.alt_name(), alt.choice_name(), arg));
+          &alt.choice(), &alt.alternative(), cast<TupleValue>(arg)));
     }
     case Value::Kind::FunctionValue: {
       const auto& fun_val = cast<FunctionValue>(*fun);

explorer/interpreter/pattern_analysis.cpp

@@ -34,7 +34,7 @@ auto AbstractPattern::discriminator() const -> std::string_view {
     }
   } else if (const auto* value = value_.dyn_cast<const Value*>()) {
     if (const auto* alt = dyn_cast<AlternativeValue>(value)) {
-      return alt->alt_name();
+      return alt->alternative().name();
     } else if (const auto* bool_val = dyn_cast<BoolValue>(value)) {
       return bool_val->value() ? "true" : "false";
     }
@@ -47,13 +47,16 @@ auto AbstractPattern::elements_size() const -> int {
     if (const auto* tuple_pattern = dyn_cast<TuplePattern>(pattern)) {
       return tuple_pattern->fields().size();
     } else if (isa<AlternativePattern>(pattern)) {
+      // Note, AlternativePattern is only used for a pattern with arguments. An
+      // alternative pattern without arguments is represented as an
+      // AlternativeValue.
       return 1;
     }
   } else if (const auto* value = value_.dyn_cast<const Value*>()) {
     if (const auto* tuple = dyn_cast<TupleValue>(value)) {
       return tuple->elements().size();
     } else if (const auto* alt = dyn_cast<AlternativeValue>(value)) {
-      return 1;
+      return alt->argument() ? 1 : 0;
     }
   }
   return 0;
@@ -78,9 +81,9 @@ void AbstractPattern::AppendElementsTo(
             AbstractPattern(tuple->elements()[i], tuple_type->elements()[i]));
       }
     } else if (const auto* alt = dyn_cast<AlternativeValue>(value)) {
-      out.push_back(AbstractPattern(
-          &alt->argument(),
-          *cast<ChoiceType>(type_)->FindAlternative(alt->alt_name())));
+      if (auto arg = alt->argument()) {
+        out.push_back(AbstractPattern(*arg, *alt->alternative().static_type()));
+      }
     }
   }
 }

explorer/interpreter/resolve_names.cpp

@@ -792,8 +792,9 @@ auto NameResolver::ResolveNames(Declaration& declaration,
       // need to check for duplicates.
       std::set<std::string_view> alternative_names;
       for (Nonnull<AlternativeSignature*> alternative : choice.alternatives()) {
-        CARBON_RETURN_IF_ERROR(
-            ResolveNames(alternative->signature(), choice_scope));
+        if (auto sig = alternative->signature()) {
+          CARBON_RETURN_IF_ERROR(ResolveNames(**sig, choice_scope));
+        }
         if (!alternative_names.insert(alternative->name()).second) {
           return ProgramError(alternative->source_loc())
                  << "Duplicate name `" << alternative->name()

explorer/interpreter/type_checker.cpp

@@ -2784,22 +2784,29 @@ auto TypeChecker::TypeCheckExp(Nonnull<Expression*> e,
             }
             case Value::Kind::ChoiceType: {
               const auto& choice = cast<ChoiceType>(*type);
-              std::optional<Nonnull<const Value*>> parameter_types =
-                  choice.FindAlternative(access.member_name());
-              if (!parameter_types.has_value()) {
+              std::optional<Nonnull<const AlternativeSignature*>> signature =
+                  choice.declaration().FindAlternative(access.member_name());
+              if (!signature.has_value()) {
                 return ProgramError(e->source_loc())
-                       << "choice " << choice.name()
-                       << " does not have an alternative named "
+                       << choice << " does not have an alternative named "
                        << access.member_name();
               }
-              Nonnull<const Value*> substituted_parameter_type =
-                  *parameter_types;
-              if (choice.IsParameterized()) {
-                substituted_parameter_type =
-                    Substitute(choice.bindings(), *parameter_types);
+
+              // If we find an alternative with no declared signature, we are
+              // constructing an unparameterized alternative value.
+              if (!(*signature)->static_type()) {
+                access.set_member(
+                    arena_->New<NamedElement>(arena_->New<NamedValue>(
+                        NamedValue{access.member_name(), &choice})));
+                access.set_static_type(&choice);
+                access.set_value_category(ValueCategory::Let);
+                return Success();
               }
-              Nonnull<const Value*> type = arena_->New<FunctionType>(
-                  substituted_parameter_type, &choice);
+
+              Nonnull<const Value*> parameter_type =
+                  Substitute(choice.bindings(), *(*signature)->static_type());
+              Nonnull<const Value*> type =
+                  arena_->New<FunctionType>(parameter_type, &choice);
               // TODO: Should there be a Declaration corresponding to each
               // choice type alternative?
               access.set_member(
@@ -3311,6 +3318,24 @@ auto TypeChecker::TypeCheckExp(Nonnull<Expression*> e,
           call.set_value_category(ValueCategory::Let);
           return Success();
         }
+        case Value::Kind::ChoiceType: {
+          // Give a better diagnostic for an attempt to call a choice constant.
+          auto* member_access =
+              dyn_cast<SimpleMemberAccessExpression>(&call.function());
+          if (member_access &&
+              isa<TypeType>(member_access->object().static_type())) {
+            CARBON_ASSIGN_OR_RETURN(
+                Nonnull<const Value*> type,
+                InterpExp(&member_access->object(), arena_, trace_stream_));
+            if (isa<ChoiceType>(type)) {
+              return ProgramError(e->source_loc())
+                     << "alternative `" << *type << "."
+                     << member_access->member_name()
+                     << "` does not expect an argument list";
+            }
+          }
+          [[fallthrough]];
+        }
         default: {
           return ProgramError(e->source_loc())
                  << "in call `" << *e
@@ -3995,26 +4020,30 @@ auto TypeChecker::TypeCheckPattern(
                                           "alternative pattern", &choice_type,
                                           *expected, impl_scope));
       }
-      std::optional<Nonnull<const Value*>> parameter_types =
-          choice_type.FindAlternative(alternative.alternative_name());
-      if (parameter_types == std::nullopt) {
+      std::optional<Nonnull<const AlternativeSignature*>> signature =
+          choice_type.declaration().FindAlternative(
+              alternative.alternative_name());
+      if (!signature) {
         return ProgramError(alternative.source_loc())
-               << "'" << alternative.alternative_name()
-               << "' is not an alternative of " << choice_type;
+               << "`" << alternative.alternative_name()
+               << "` is not an alternative of " << choice_type;
       }
-
-      Nonnull<const Value*> substituted_parameter_type = *parameter_types;
-      if (choice_type.IsParameterized()) {
-        substituted_parameter_type =
-            Substitute(choice_type.bindings(), *parameter_types);
+      if (!(*signature)->static_type()) {
+        return ProgramError(alternative.source_loc())
+               << "alternative `" << choice_type << "."
+               << alternative.alternative_name()
+               << "` does not expect an argument list";
       }
-      CARBON_RETURN_IF_ERROR(
-          TypeCheckPattern(&alternative.arguments(), substituted_parameter_type,
-                           impl_scope, enclosing_value_category));
+
+      Nonnull<const Value*> parameter_type =
+          Substitute(choice_type.bindings(), *(*signature)->static_type());
+      CARBON_RETURN_IF_ERROR(TypeCheckPattern(&alternative.arguments(),
+                                              parameter_type, impl_scope,
+                                              enclosing_value_category));
       alternative.set_static_type(&choice_type);
       alternative.set_value(arena_->New<AlternativeValue>(
-          alternative.alternative_name(), choice_type.name(),
-          &alternative.arguments().value()));
+          &choice_type, *signature,
+          cast<TupleValue>(&alternative.arguments().value())));
       return Success();
     }
     case PatternKind::ExpressionPattern: {
@@ -5468,14 +5497,14 @@ auto TypeChecker::DeclareChoiceDeclaration(Nonnull<ChoiceDeclaration*> choice,
     }
   }
 
-  std::vector<NamedValue> alternatives;
   for (Nonnull<AlternativeSignature*> alternative : choice->alternatives()) {
-    CARBON_ASSIGN_OR_RETURN(auto signature,
-                            TypeCheckTypeExp(&alternative->signature(),
-                                             *scope_info.innermost_scope));
-    alternatives.push_back({alternative->name(), signature});
+    if (auto signature = alternative->signature()) {
+      CARBON_ASSIGN_OR_RETURN(
+          auto type, TypeCheckTypeExp(*signature, *scope_info.innermost_scope));
+      alternative->set_static_type(type);
+    }
   }
-  choice->set_members(alternatives);
+
   if (choice->type_params().has_value()) {
     Nonnull<ParameterizedEntityName*> param_name =
         arena_->New<ParameterizedEntityName>(choice, *choice->type_params());

explorer/interpreter/value.cpp

@@ -195,11 +195,15 @@ static auto GetNamedElement(Nonnull<Arena*> arena, Nonnull<const Value*> v,
     }
     case Value::Kind::ChoiceType: {
       const auto& choice = cast<ChoiceType>(*v);
-      if (!choice.FindAlternative(f)) {
+      auto alt = choice.declaration().FindAlternative(f);
+      if (!alt) {
         return ProgramError(source_loc)
                << "alternative " << f << " not in " << *v;
       }
-      return arena->New<AlternativeConstructorValue>(f, choice.name());
+      if ((*alt)->signature()) {
+        return arena->New<AlternativeConstructorValue>(&choice, *alt);
+      }
+      return arena->New<AlternativeValue>(&choice, *alt, std::nullopt);
     }
     case Value::Kind::NominalClassType: {
       // Access a class function.
@@ -360,7 +364,8 @@ void Value::Print(llvm::raw_ostream& out) const {
   switch (kind()) {
     case Value::Kind::AlternativeConstructorValue: {
       const auto& alt = cast<AlternativeConstructorValue>(*this);
-      out << alt.choice_name() << "." << alt.alt_name();
+      out << alt.choice().declaration().name() << "."
+          << alt.alternative().name();
       break;
     }
     case Value::Kind::BindingPlaceholderValue: {
@@ -381,8 +386,11 @@ void Value::Print(llvm::raw_ostream& out) const {
     }
     case Value::Kind::AlternativeValue: {
       const auto& alt = cast<AlternativeValue>(*this);
-      out << "alt " << alt.choice_name() << "." << alt.alt_name() << " "
-          << alt.argument();
+      out << alt.choice().declaration().name() << "."
+          << alt.alternative().name();
+      if (auto arg = alt.argument()) {
+        out << **arg;
+      }
       break;
     }
     case Value::Kind::StructValue: {
@@ -536,6 +544,13 @@ void Value::Print(llvm::raw_ostream& out) const {
       }
       break;
     }
+    case Value::Kind::ChoiceType: {
+      const auto& choice_type = cast<ChoiceType>(*this);
+      out << "choice ";
+      PrintNameWithBindings(out, &choice_type.declaration(),
+                            choice_type.type_args());
+      break;
+    }
     case Value::Kind::MixinPseudoType: {
       const auto& mixin_type = cast<MixinPseudoType>(*this);
       out << "mixin ";
@@ -647,9 +662,6 @@ void Value::Print(llvm::raw_ostream& out) const {
       }
       break;
     }
-    case Value::Kind::ChoiceType:
-      out << "choice " << cast<ChoiceType>(*this).name();
-      break;
     case Value::Kind::VariableType:
       out << cast<VariableType>(*this).binding().name();
       break;
@@ -1013,6 +1025,17 @@ auto ValueStructurallyEqual(
       }
       return true;
     }
+    case Value::Kind::AlternativeValue: {
+      const auto& alt1 = cast<AlternativeValue>(*v1);
+      const auto& alt2 = cast<AlternativeValue>(*v2);
+      if (!TypeEqual(&alt1.choice(), &alt2.choice(), equality_ctx) ||
+          &alt1.alternative() != &alt2.alternative()) {
+        return false;
+      }
+      CARBON_CHECK(alt1.argument().has_value() == alt2.argument().has_value());
+      return !alt1.argument().has_value() ||
+             ValueEqual(*alt1.argument(), *alt2.argument(), equality_ctx);
+    }
     case Value::Kind::StringValue:
       return cast<StringValue>(*v1).value() == cast<StringValue>(*v2).value();
     case Value::Kind::ParameterizedEntityName: {
@@ -1059,7 +1082,6 @@ auto ValueStructurallyEqual(
     case Value::Kind::StaticArrayType:
       return TypeEqual(v1, v2, equality_ctx);
     case Value::Kind::NominalClassValue:
-    case Value::Kind::AlternativeValue:
     case Value::Kind::BindingPlaceholderValue:
     case Value::Kind::AddrValue:
     case Value::Kind::AlternativeConstructorValue:
@@ -1151,17 +1173,6 @@ auto ConstraintType::VisitEqualValues(
   return true;
 }
 
-auto ChoiceType::FindAlternative(std::string_view name) const
-    -> std::optional<Nonnull<const Value*>> {
-  std::vector<NamedValue> alternatives = declaration_->members();
-  for (const NamedValue& alternative : alternatives) {
-    if (alternative.name == name) {
-      return alternative.value;
-    }
-  }
-  return std::nullopt;
-}
-
 auto FindFunction(std::string_view name,
                   llvm::ArrayRef<Nonnull<Declaration*>> members)
     -> std::optional<Nonnull<const FunctionValue*>> {

explorer/interpreter/value.h

@@ -26,6 +26,8 @@ namespace Carbon {
 
 class Action;
 class AssociatedConstant;
+class ChoiceType;
+class TupleValue;
 
 // A trait type that describes how to allocate an instance of `T` in an arena.
 // Returns the created object, which is not required to be of type `T`.
@@ -379,16 +381,30 @@ class NominalClassValue : public Value {
   Nonnull<const NominalClassValue** const> class_value_ptr_;
 };
 
+// Common implementation of alternative values and alternative constructors.
+class AlternativeValueBase : public Value {
+ public:
+  AlternativeValueBase(Kind kind, Nonnull<const ChoiceType*> choice,
+                       Nonnull<const AlternativeSignature*> alternative)
+      : Value(kind), choice_(choice), alternative_(alternative) {}
+
+  auto choice() const -> const ChoiceType& { return *choice_; }
+  auto alternative() const -> const AlternativeSignature& {
+    return *alternative_;
+  }
+
+ private:
+  Nonnull<const ChoiceType*> choice_;
+  Nonnull<const AlternativeSignature*> alternative_;
+};
+
 // An alternative constructor value.
-// TODO: The representation here is inappropriate: at least the choice type
-// should be identified symbolically rather than by name.
-class AlternativeConstructorValue : public Value {
+class AlternativeConstructorValue : public AlternativeValueBase {
  public:
-  AlternativeConstructorValue(std::string_view alt_name,
-                              std::string_view choice_name)
-      : Value(Kind::AlternativeConstructorValue),
-        alt_name_(alt_name),
-        choice_name_(choice_name) {}
+  AlternativeConstructorValue(Nonnull<const ChoiceType*> choice,
+                              Nonnull<const AlternativeSignature*> alternative)
+      : AlternativeValueBase(Kind::AlternativeConstructorValue, choice,
+                             alternative) {}
 
   static auto classof(const Value* value) -> bool {
     return value->kind() == Kind::AlternativeConstructorValue;
@@ -396,27 +412,17 @@ class AlternativeConstructorValue : public Value {
 
   template <typename F>
   auto Decompose(F f) const {
-    return f(alt_name_, choice_name_);
+    return f(&choice(), &alternative());
   }
-
-  auto alt_name() const -> const std::string& { return alt_name_; }
-  auto choice_name() const -> const std::string& { return choice_name_; }
-
- private:
-  std::string alt_name_;
-  std::string choice_name_;
 };
 
 // An alternative value.
-// TODO: The representation here is inappropriate: at least the choice type
-// should be identified symbolically rather than by name.
-class AlternativeValue : public Value {
+class AlternativeValue : public AlternativeValueBase {
  public:
-  AlternativeValue(std::string_view alt_name, std::string_view choice_name,
-                   Nonnull<const Value*> argument)
-      : Value(Kind::AlternativeValue),
-        alt_name_(alt_name),
-        choice_name_(choice_name),
+  AlternativeValue(Nonnull<const ChoiceType*> choice,
+                   Nonnull<const AlternativeSignature*> alternative,
+                   std::optional<Nonnull<const TupleValue*>> argument)
+      : AlternativeValueBase(Kind::AlternativeValue, choice, alternative),
         argument_(argument) {}
 
   static auto classof(const Value* value) -> bool {
@@ -425,17 +431,15 @@ class AlternativeValue : public Value {
 
   template <typename F>
   auto Decompose(F f) const {
-    return f(alt_name_, choice_name_, argument_);
+    return f(&choice(), &alternative(), argument_);
   }
 
-  auto alt_name() const -> const std::string& { return alt_name_; }
-  auto choice_name() const -> const std::string& { return choice_name_; }
-  auto argument() const -> const Value& { return *argument_; }
+  auto argument() const -> std::optional<Nonnull<const TupleValue*>> {
+    return argument_;
+  }
 
  private:
-  std::string alt_name_;
-  std::string choice_name_;
-  Nonnull<const Value*> argument_;
+  std::optional<Nonnull<const TupleValue*>> argument_;
 };
 
 // Base class for tuple types and tuple values. These are the same other than
@@ -1254,16 +1258,6 @@ class ChoiceType : public Value {
     return f(declaration_, bindings_);
   }
 
-  // TODO: Remove this.
-  auto name() const -> std::string_view {
-    return declaration_->name().inner_name();
-  }
-
-  // Returns the parameter types of the alternative with the given name,
-  // or nullopt if no such alternative is present.
-  auto FindAlternative(std::string_view name) const
-      -> std::optional<Nonnull<const Value*>>;
-
   auto bindings() const -> const Bindings& { return *bindings_; }
 
   auto type_args() const -> const BindingMap& { return bindings_->args(); }

explorer/syntax/parser.ypp

@@ -1181,9 +1181,8 @@ alternative:
     { $$ = arena->New<AlternativeSignature>(context.source_loc(), $1, $2); }
 | identifier
     {
-      $$ = arena->New<AlternativeSignature>(
-          context.source_loc(), $1,
-          arena->New<TupleLiteral>(context.source_loc()));
+      $$ = arena->New<AlternativeSignature>(context.source_loc(), $1,
+                                            std::nullopt);
     }
 ;
 alternative_list:

explorer/testdata/basic_syntax/choice.carbon

@@ -5,7 +5,7 @@
 // AUTOUPDATE
 // RUN: %{explorer-run}
 // RUN: %{explorer-run-trace}
-// CHECK:STDOUT: result: 0
+// CHECK:STDOUT: result: 1
 
 package ExplorerTest api;
 
@@ -15,26 +15,30 @@ choice Ints {
   Two(i32, i32)
 }
 
+// Test some alternate syntaxes
+choice MoreInts {
+  None(),
+  One(i32),
+  Two(i32, i32),
+}
+
 fn Main() -> i32 {
-  var x: Ints = Ints.None();
+  var x: Ints = Ints.None;
   var y: Ints = Ints.One(42);
+  var z: MoreInts = MoreInts.None();
   var n: i32 = 0;
   match (y) {
-    case Ints.None() => { n = n + 2; }
+    case Ints.None => { n = n + 2; }
     case Ints.One(x: auto) => { n = x + 1 - 42; }
     case Ints.Two(a: auto, b: auto) => { n = 2; }
   }
   match (x) {
     case Ints.One(x: auto) => { n = x + 2; }
-    case Ints.None() => { n = n - 1; }
+    case Ints.None => { n = n - 1; }
     case Ints.Two(x: auto, y: auto) => { n = 5; }
   }
+  match (z) {
+    case MoreInts.None() => { ++n; }
+  }
   return n;
 }
-
-// Test some alternate syntaxes
-choice MoreInts {
-  None(),
-  One(i32),
-  Two(i32, i32),
-}

explorer/testdata/basic_syntax/fail_choice_extra_parens.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
+//
+// AUTOUPDATE
+// RUN: %{not} %{explorer-run}
+// RUN: %{not} %{explorer-run-trace}
+
+package ExplorerTest api;
+
+choice Ints {
+  None,
+  One(i32),
+  Two(i32, i32)
+}
+
+fn Main() -> i32 {
+  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/basic_syntax/fail_choice_extra_parens.carbon:[[@LINE+1]]: alternative `choice Ints.None` does not expect an argument list
+  match (Ints.None()) {
+    case Ints.None => { return 0; }
+    default => { return 1; }
+  }
+}

explorer/testdata/basic_syntax/fail_choice_no_parens.carbon

@@ -9,17 +9,17 @@
 package ExplorerTest api;
 
 choice Ints {
-  None,
+  None(),
   One(i32),
   Two(i32, i32)
 }
 
 fn Main() -> i32 {
-  match (Ints.None()) {
-    case Ints.None => { return 0; }
+  match (Ints.None) {
+    case Ints.None() => { return 0; }
     // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/basic_syntax/fail_choice_no_parens.carbon:[[@LINE+3]]: type error in `match` pattern type
-    // CHECK:STDERR: expected: fn () -> choice Ints
-    // CHECK:STDERR: actual: choice Ints
+    // CHECK:STDERR: expected: choice Ints
+    // CHECK:STDERR: actual: fn () -> choice Ints
     default => { return 1; }
   }
 }

explorer/testdata/basic_syntax/fail_choice_pattern_extra_parens.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
+//
+// AUTOUPDATE
+// RUN: %{not} %{explorer-run}
+// RUN: %{not} %{explorer-run-trace}
+
+package ExplorerTest api;
+
+choice Ints {
+  None,
+  One(i32),
+  Two(i32, i32)
+}
+
+fn Main() -> i32 {
+  match (Ints.None) {
+    // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/basic_syntax/fail_choice_pattern_extra_parens.carbon:[[@LINE+1]]: alternative `choice Ints.None` does not expect an argument list
+    case Ints.None() => { return 0; }
+    default => { return 1; }
+  }
+}

explorer/testdata/basic_syntax/fail_choice_pattern_no_parens.carbon

@@ -0,0 +1,25 @@
+// 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
+//
+// AUTOUPDATE
+// RUN: %{not} %{explorer-run}
+// RUN: %{not} %{explorer-run-trace}
+
+package ExplorerTest api;
+
+choice Ints {
+  None(),
+  One(i32),
+  Two(i32, i32)
+}
+
+fn Main() -> i32 {
+  match (Ints.None()) {
+    case Ints.None => { return 0; }
+    // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/basic_syntax/fail_choice_pattern_no_parens.carbon:[[@LINE+3]]: type error in `match` pattern type
+    // CHECK:STDERR: expected: fn () -> choice Ints
+    // CHECK:STDERR: actual: choice Ints
+    default => { return 1; }
+  }
+}

explorer/testdata/let/fail_match_choice.carbon

@@ -15,13 +15,13 @@ choice Ints {
 }
 
 fn Main() -> i32 {
-  var x: auto = Ints.None();
+  var x: auto = Ints.None;
   var n: auto = 0;
   match (x) {
     case Ints.One(var x: auto) => {
       x = 2;
     }
-    case Ints.None() => {
+    case Ints.None => {
       n = n - 1;
     }
     case Ints.Two(x: auto, y: auto) => {