From 694dd7bf5588720bd64e0320033ceed64ecc0564 Mon Sep 17 00:00:00 2001
From: Jon Ross-Perkins <jperkins@google.com>
Date: Wed, 12 Apr 2023 15:56:27 -0700
Subject: [PATCH] Fix poor dyn_cast usage (#2763)

Note the dest_class case didn't actually check dyn_cast success until after using the pointer. That's the case I noticed first, but also cleaning up a couple other small related things.
---
 explorer/ast/element.cpp              |  4 ++--
 explorer/interpreter/interpreter.cpp  |  2 +-
 explorer/interpreter/type_checker.cpp | 15 +++++----------
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/explorer/ast/element.cpp b/explorer/ast/element.cpp
index d4c35ed89984..0326828629f2 100644
--- a/explorer/ast/element.cpp
+++ b/explorer/ast/element.cpp
@@ -22,7 +22,7 @@ auto NamedElement::name() const -> std::string_view {
   if (const auto* decl = element_.dyn_cast<const Declaration*>()) {
     return GetName(*decl).value();
   } else {
-    const auto* named_value = element_.dyn_cast<const NamedValue*>();
+    const auto* named_value = element_.get<const NamedValue*>();
     return named_value->name;
   }
 }
@@ -31,7 +31,7 @@ auto NamedElement::type() const -> const Value& {
   if (const auto* decl = element_.dyn_cast<const Declaration*>()) {
     return decl->static_type();
   } else {
-    const auto* named_value = element_.dyn_cast<const NamedValue*>();
+    const auto* named_value = element_.get<const NamedValue*>();
     return *named_value->value;
   }
 }
diff --git a/explorer/interpreter/interpreter.cpp b/explorer/interpreter/interpreter.cpp
index c0be03a00d2d..53f1e0caf714 100644
--- a/explorer/interpreter/interpreter.cpp
+++ b/explorer/interpreter/interpreter.cpp
@@ -979,7 +979,7 @@ auto Interpreter::CallFunction(const CallExpression& call,
     }
     case Value::Kind::FunctionValue:
     case Value::Kind::BoundMethodValue: {
-      const auto* func_val = dyn_cast<FunctionOrMethodValue>(fun);
+      const auto* func_val = cast<FunctionOrMethodValue>(fun);
 
       const FunctionDeclaration& function = func_val->declaration();
       if (!function.body().has_value()) {
diff --git a/explorer/interpreter/type_checker.cpp b/explorer/interpreter/type_checker.cpp
index 364c2f9082ce..97ebed3dcbaf 100644
--- a/explorer/interpreter/type_checker.cpp
+++ b/explorer/interpreter/type_checker.cpp
@@ -711,12 +711,9 @@ auto TypeChecker::BuildSubtypeConversion(Nonnull<Expression*> source,
                                          Nonnull<const PointerType*> src_ptr,
                                          Nonnull<const PointerType*> dest_ptr)
     -> ErrorOr<Nonnull<const Expression*>> {
-  const auto* src_class = dyn_cast<NominalClassType>(&src_ptr->pointee_type());
-  const auto* dest_class =
-      dyn_cast<NominalClassType>(&dest_ptr->pointee_type());
+  const auto* src_class = cast<NominalClassType>(&src_ptr->pointee_type());
+  const auto* dest_class = cast<NominalClassType>(&dest_ptr->pointee_type());
   const auto dest = dest_class->declaration().name();
-  CARBON_CHECK(src_class && dest_class)
-      << "Invalid source or destination pointee";
   Nonnull<Expression*> last_expr = source;
   const auto* cur_class = src_class;
   while (!TypeEqual(cur_class, dest_class, std::nullopt)) {
@@ -6040,9 +6037,7 @@ auto TypeChecker::TypeCheckDeclaration(
       if (var.has_initializer()) {
         CARBON_RETURN_IF_ERROR(TypeCheckExp(&var.initializer(), impl_scope));
       }
-      const auto* binding_type =
-          dyn_cast<ExpressionPattern>(&var.binding().type());
-      if (binding_type == nullptr) {
+      if (!isa<ExpressionPattern>(&var.binding().type())) {
         // TODO: consider adding support for `auto`
         return ProgramError(var.source_loc())
                << "Type of a top-level variable must be an expression.";
@@ -6153,7 +6148,7 @@ auto TypeChecker::DeclareDeclaration(Nonnull<Declaration*> d,
       auto& var = cast<VariableDeclaration>(*d);
       // Associate the variable name with it's declared type in the
       // compile-time symbol table.
-      if (!llvm::isa<ExpressionPattern>(var.binding().type())) {
+      if (!isa<ExpressionPattern>(var.binding().type())) {
         return ProgramError(var.binding().type().source_loc())
                << "Expected expression for variable type";
       }
@@ -6213,7 +6208,7 @@ auto TypeChecker::FindMixedMemberAndType(
     -> ErrorOr<std::optional<
         std::pair<Nonnull<const Value*>, Nonnull<const Declaration*>>>> {
   for (Nonnull<const Declaration*> member : members) {
-    if (llvm::isa<MixDeclaration>(member)) {
+    if (isa<MixDeclaration>(member)) {
       const auto& mix_decl = cast<MixDeclaration>(*member);
       Nonnull<const MixinPseudoType*> mixin = &mix_decl.mixin_value();
       CARBON_ASSIGN_OR_RETURN(