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

Code before:
                                /*IsReinject=*/false);
  parser.ConsumeAnyToken(true);

  clang::ExprResult result = parser.ParseConstantExpression();
  clang::Expr* result_expr = result.get();

  bool success =
      !result.isInvalid() && parser.getCurToken().is(clang::tok::eof);

  if (!success) {
    parser.SkipUntil(clang::tok::eof);
    CARBON_DIAGNOSTIC(
        InCppMacroEvaluation, Error,
        "failed to parse macro Cpp.{0} to a valid constant expression",
        std::string);
    context.emitter().Emit(loc_id, InCppMacroEvaluation, (*name_str_opt).str());
    return nullptr;
  }

  clang::Expr::EvalResult evaluated_result;
  if (!result_expr->EvaluateAsConstantExpr(evaluated_result,
                                           sema.getASTContext())) {
    context.TODO(loc_id,
                 "failed to evaluate macro to a valid constant expression");
    return nullptr;
  }
  clang::APValue ap_value = evaluated_result.Val;
  if (ap_value.isInt()) {
    return clang::IntegerLiteral::Create(
        sema.getASTContext(), ap_value.getInt(), result_expr->getType(),
        result_expr->getExprLoc());
  }
  if (ap_value.isFloat()) {
    return clang::FloatingLiteral::Create(
        sema.getASTContext(), ap_value.getFloat(),
        /*isExact=*/true, result_expr->getType(), result_expr->getExprLoc());
  }
  context.TODO(loc_id, "Unsupported: constant type:" +
                           result_expr->getType().getAsString());
  return nullptr;
}

}  // namespace Carbon::Check


Review comment (suggestion):
Given the switch to `EvaluateAsConstantExpr` combined with `ParseConstantExpression` above, is there a way to write a test that can reach the TODO? If it seems to be unreachable by tests, maybe make this a CARBON_CHECK, i.e.:

```suggestion
  CARBON_CHECK(result_expr->EvaluateAsConstantExpr(evaluated_result,
                                           sema.getASTContext()));
```

Granted someone may figure out a way to reach this, but then we can also use that to write a test to verify handling. :)

Code after:
                                /*IsReinject=*/false);
  parser.ConsumeAnyToken(true);

  clang::ExprResult result = parser.ParseConstantExpression();
  clang::Expr* result_expr = result.get();

  bool success =
      !result.isInvalid() && parser.getCurToken().is(clang::tok::eof);

  if (!success) {
    parser.SkipUntil(clang::tok::eof);
    CARBON_DIAGNOSTIC(
        InCppMacroEvaluation, Error,
        "failed to parse macro Cpp.{0} to a valid constant expression",
        std::string);
    context.emitter().Emit(loc_id, InCppMacroEvaluation, (*name_str_opt).str());
    return nullptr;
  }

  clang::Expr::EvalResult evaluated_result;
  CARBON_CHECK(result_expr->EvaluateAsConstantExpr(evaluated_result,
                                                   sema.getASTContext()));
  clang::APValue ap_value = evaluated_result.Val;
  switch (ap_value.getKind()) {
    case clang::APValue::Int:
      return clang::IntegerLiteral::Create(
          sema.getASTContext(), ap_value.getInt(), result_expr->getType(),
          result_expr->getExprLoc());
    case clang::APValue::Float:
      return clang::FloatingLiteral::Create(
          sema.getASTContext(), ap_value.getFloat(),
          /*isExact=*/true, result_expr->getType(), result_expr->getExprLoc());
    default:
      context.TODO(loc_id,
                   "Unsupported: macro evaluated to a constant of type: " +
                       result_expr->getType().getAsString());
      return nullptr;
  }
}

}  // namespace Carbon::Check