Repository: carbon-language/carbon-lang
File: toolchain/check/eval.cpp (C++)

Code before:

// Returns a constant for a call to a builtin function.
static auto MakeConstantForBuiltinCall(EvalContext& eval_context, SemIRLoc loc,
                                       SemIR::Call call,
                                       SemIR::BuiltinFunctionKind builtin_kind,
                                       llvm::ArrayRef<SemIR::InstId> arg_ids,
                                       Phase phase) -> SemIR::ConstantId {
  auto& context = eval_context.context();
  switch (builtin_kind) {
    case SemIR::BuiltinFunctionKind::None:
      CARBON_FATAL("Not a builtin function.");

    case SemIR::BuiltinFunctionKind::PrintChar:
    case SemIR::BuiltinFunctionKind::PrintInt:
    case SemIR::BuiltinFunctionKind::ReadChar: {
      // These are runtime-only builtins.
      // TODO: Consider tracking this on the `BuiltinFunctionKind`.
      return SemIR::ConstantId::NotConstant;
    }

    case SemIR::BuiltinFunctionKind::TypeAnd: {
      CARBON_CHECK(arg_ids.size() == 2);
      auto facet_type_ids = std::to_array<SemIR::FacetTypeId>({
          SemIR::FacetTypeId::None,
          SemIR::FacetTypeId::None,
      });
      for (auto [i, arg_id] : llvm::enumerate(arg_ids)) {
        if (auto facet_type =
                context.insts().TryGetAs<SemIR::FacetType>(arg_id)) {
          facet_type_ids[i] = facet_type->facet_type_id;
        } else {
          CARBON_DIAGNOSTIC(
              FacetTypeRequiredForTypeAndOperator, Error,
              "BitAnd operator on types requires facet type on both sides");
          context.emitter().Emit(loc, FacetTypeRequiredForTypeAndOperator);
        }
      }
      // Allow errors to be diagnosed for both sides of the operator before
      // returning here if any error occurred on either side.
      if (llvm::any_of(facet_type_ids,
                       [](SemIR::FacetTypeId id) { return !id.has_value(); })) {
        return SemIR::ConstantId::NotConstant;
      }
      // Reuse one of the argument instructions if nothing has changed.
      if (facet_type_ids[0] == facet_type_ids[1]) {
        return context.types().GetConstantId(
            context.types().GetTypeIdForTypeInstId(arg_ids[0]));
      }
      auto info = SemIR::FacetTypeInfo::Combined(
          context.facet_types().Get(facet_type_ids[0]),
          context.facet_types().Get(facet_type_ids[1]));

Review comment (suggestion):
I'm trying to understand why you're using arrays here. Also, why do you use `llvm::any_of` on line 1346 when there are only two values, versus `!facet_type_ids[0].has_value() || !facet_type_ids[1].has_value()`?

If this is mainly for code-sharing of 1334-1342, had you considered instead declaring `lhs` and `rhs`, and either something like:

```
for (auto [facet_type_id, arg_id] : llvm::zip(std::to_array({&lhs_id, &rhs_id}), arg_ids)) {
```

Or a lambda like:

```
auto get_facet_type_id = [](SemIR::InstId arg_id) ...
auto lhs_id = get_facet_type_id(arg_ids[0]);
auto rhs_id = get_facet_type_id(arg_ids[1]);
```

(note, I commented on a prior PR about single-use lambdas, but here it'd support code reuse)

Code after:
                                       llvm::ArrayRef<SemIR::InstId> arg_ids,
                                       Phase phase) -> SemIR::ConstantId {
  auto& context = eval_context.context();
  switch (builtin_kind) {
    case SemIR::BuiltinFunctionKind::None:
      CARBON_FATAL("Not a builtin function.");

    case SemIR::BuiltinFunctionKind::PrintChar:
    case SemIR::BuiltinFunctionKind::PrintInt:
    case SemIR::BuiltinFunctionKind::ReadChar: {
      // These are runtime-only builtins.
      // TODO: Consider tracking this on the `BuiltinFunctionKind`.
      return SemIR::ConstantId::NotConstant;
    }

    case SemIR::BuiltinFunctionKind::TypeAnd: {
      CARBON_CHECK(arg_ids.size() == 2);
      auto lhs_facet_type_id = SemIR::FacetTypeId::None;
      auto rhs_facet_type_id = SemIR::FacetTypeId::None;
      for (auto [facet_type_id, arg_id] :
           llvm::zip(std::to_array({&lhs_facet_type_id, &rhs_facet_type_id}),
                     arg_ids)) {
        if (auto facet_type =
                context.insts().TryGetAs<SemIR::FacetType>(arg_id)) {
          *facet_type_id = facet_type->facet_type_id;
        } else {
          CARBON_DIAGNOSTIC(FacetTypeRequiredForTypeAndOperator, Error,
                            "non-facet type {0} combined with `&` operator",
                            SemIR::TypeId);
          // TODO: Find a location for the lhs or rhs specifically, instead of
          // the whole thing. If that's not possible we can change the text to
          // say if it's referring to the left or the right side for the error.
          // The `arg_id` instruction has no location in it for some reason.
          context.emitter().Emit(
              loc, FacetTypeRequiredForTypeAndOperator,
              context.types().GetTypeIdForTypeInstId(arg_id));
        }
      }
      // Allow errors to be diagnosed for both sides of the operator before
      // returning here if any error occurred on either side.
      if (!lhs_facet_type_id.has_value() || !rhs_facet_type_id.has_value()) {
        return SemIR::ErrorInst::SingletonConstantId;
      }
      // Reuse one of the argument instructions if nothing has changed.
      if (lhs_facet_type_id == rhs_facet_type_id) {
        return context.types().GetConstantId(
            context.types().GetTypeIdForTypeInstId(arg_ids[0]));
      }
      auto info = SemIR::FacetTypeInfo::Combine(
          context.facet_types().Get(lhs_facet_type_id),
          context.facet_types().Get(rhs_facet_type_id));