Support constexpr pointers (#6907)

This moves the LValue path code from macros.cpp to constant.cpp, so that
it can be called from `MapAPValueToConstant`. TODO messages are updated
accordingly to avoid referring to macros. Added a constexpr pointer test
to `constexpr.carbon` to show the result of this change.

toolchain/check/cpp/constant.cpp

@@ -5,20 +5,105 @@
 #include "toolchain/check/cpp/constant.h"
 
 #include "toolchain/check/cpp/import.h"
+#include "toolchain/check/cpp/type_mapping.h"
 #include "toolchain/check/eval.h"
+#include "toolchain/check/member_access.h"
+#include "toolchain/check/type.h"
+#include "toolchain/check/type_completion.h"
 #include "toolchain/diagnostics/format_providers.h"
 
 namespace Carbon::Check {
 
+static auto MapLValueToConstant(Context& context, SemIR::LocId loc_id,
+                                const clang::APValue& ap_value,
+                                clang::QualType type) -> SemIR::ConstantId {
+  CARBON_CHECK(ap_value.isLValue(), "not an LValue");
+
+  const auto* value_decl =
+      ap_value.getLValueBase().get<const clang::ValueDecl*>();
+
+  if (!ap_value.hasLValuePath()) {
+    context.TODO(loc_id, "lvalue has no path");
+    return SemIR::ErrorInst::ConstantId;
+  }
+
+  if (ap_value.isLValueOnePastTheEnd()) {
+    context.TODO(loc_id, "one-past-the-end lvalue");
+    return SemIR::ErrorInst::ConstantId;
+  }
+
+  auto key = SemIR::ClangDeclKey::ForNonFunctionDecl(
+      // TODO: can this const_cast be avoided?
+      const_cast<clang::ValueDecl*>(value_decl));
+
+  auto inst_id = ImportCppDecl(context, loc_id, key);
+  if (ap_value.getLValuePath().size() == 0) {
+    return context.constant_values().Get(inst_id);
+  }
+
+  // Import the base type so that its fields can be accessed.
+  auto var_storage = context.insts().GetAs<SemIR::VarStorage>(inst_id);
+  // TODO: currently an error isn't reachable here because incomplete
+  // array types can't be imported. Once that changes, switch to
+  // `RequireCompleteType` and handle the error.
+  CompleteTypeOrCheckFail(context, var_storage.type_id);
+
+  clang::QualType qual_type = ap_value.getLValueBase().getType();
+  for (const auto& entry : ap_value.getLValuePath()) {
+    if (qual_type->isArrayType()) {
+      context.TODO(loc_id, "lvalue path contains an array type");
+    } else {
+      const auto* decl =
+          cast<clang::Decl>(entry.getAsBaseOrMember().getPointer());
+
+      const auto* field_decl = dyn_cast<clang::FieldDecl>(decl);
+      if (!field_decl) {
+        context.TODO(loc_id, "lvalue path contains a base class subobject");
+        return SemIR::ErrorInst::ConstantId;
+      }
+
+      auto field_inst_id =
+          ImportCppDecl(context, loc_id,
+                        SemIR::ClangDeclKey::ForNonFunctionDecl(
+                            const_cast<clang::FieldDecl*>(field_decl)));
+
+      if (field_inst_id == SemIR::ErrorInst::InstId) {
+        context.TODO(loc_id,
+                     "unsupported field in lvalue path: " +
+                         ap_value.getAsString(context.ast_context(), type));
+        return SemIR::ErrorInst::ConstantId;
+      }
+
+      const SemIR::FieldDecl& field_decl_inst =
+          context.insts().GetAs<SemIR::FieldDecl>(field_inst_id);
+
+      qual_type = field_decl->getType();
+      inst_id = PerformMemberAccess(context, loc_id, inst_id,
+                                    field_decl_inst.name_id);
+    }
+  }
+
+  return context.constant_values().Get(inst_id);
+}
+
 auto MapAPValueToConstant(Context& context, SemIR::LocId loc_id,
-                          const clang::APValue& ap_value, clang::QualType type)
-    -> SemIR::ConstantId {
+                          const clang::APValue& ap_value, clang::QualType type,
+                          bool is_lvalue) -> SemIR::ConstantId {
   SemIR::TypeId type_id = ImportCppType(context, loc_id, type).type_id;
   if (!type_id.has_value()) {
     return SemIR::ConstantId::NotConstant;
   }
 
-  if (ap_value.isInt()) {
+  if (is_lvalue) {
+    return MapLValueToConstant(context, loc_id, ap_value, type);
+  } else if (type->isPointerType()) {
+    auto const_id = MapLValueToConstant(context, loc_id, ap_value, type);
+    auto inst_id = AddInst<SemIR::AddrOf>(
+        context, loc_id,
+        {.type_id = type_id,
+         .lvalue_id = context.constant_values().GetInstId(const_id)});
+    return context.constant_values().Get(inst_id);
+  } else if (ap_value.isInt()) {
     if (type->isBooleanType()) {
       auto value = SemIR::BoolValue::From(!ap_value.getInt().isZero());
       return TryEvalInst(
@@ -36,10 +121,45 @@ auto MapAPValueToConstant(Context& context, SemIR::LocId loc_id,
         context, SemIR::FloatValue{.type_id = type_id, .float_id = float_id});
   } else {
     // TODO: support other types.
-    return SemIR::ConstantId::NotConstant;
+    context.TODO(loc_id, "unsupported conversion to constant from APValue " +
+                             ap_value.getAsString(context.ast_context(), type));
+    return SemIR::ErrorInst::ConstantId;
   }
 }
 
+static auto MapAPValueToConstantForConstexpr(Context& context,
+                                             SemIR::LocId loc_id,
+                                             const clang::APValue& ap_value,
+                                             clang::QualType type)
+    -> SemIR::ConstantId {
+  bool is_lvalue = false;
+  if (type->isReferenceType()) {
+    is_lvalue = true;
+    type = type.getNonReferenceType();
+  }
+  return MapAPValueToConstant(context, loc_id, ap_value, type, is_lvalue);
+}
+
+auto EvalCppVarDecl(Context& context, SemIR::LocId loc_id,
+                    const clang::VarDecl* var_decl, SemIR::TypeId type_id)
+    -> SemIR::ConstantId {
+  // If the C++ global is constant, map it to a Carbon constant.
+  if (var_decl->isUsableInConstantExpressions(context.ast_context())) {
+    if (const auto* ap_value = var_decl->getEvaluatedValue()) {
+      auto clang_type = MapToCppType(context, type_id);
+      if (clang_type.isNull()) {
+        context.TODO(loc_id, "failed to map C++ type to Carbon");
+        return SemIR::ErrorInst::ConstantId;
+      }
+
+      return MapAPValueToConstantForConstexpr(context, loc_id, *ap_value,
+                                              clang_type);
+    }
+  }
+
+  return SemIR::ConstantId::NotConstant;
+}
+
 static auto ConvertConstantToAPValue(Context& context,
                                      SemIR::InstId const_inst_id,
                                      clang::QualType param_type)
@@ -136,8 +256,9 @@ auto EvalCppCall(Context& context, SemIR::LocId loc_id,
                            function_decl->isConsteval());
     return SemIR::ErrorInst::ConstantId;
   }
-  return MapAPValueToConstant(context, loc_id, eval_result.Val,
-                              function_decl->getCallResultType());
+
+  return MapAPValueToConstantForConstexpr(context, loc_id, eval_result.Val,
+                                          function_decl->getCallResultType());
 }
 
 }  // namespace Carbon::Check

toolchain/check/cpp/constant.h

@@ -13,7 +13,12 @@ namespace Carbon::Check {
 
 // Converts an `APValue` to a Carbon `ConstantId`.
 auto MapAPValueToConstant(Context& context, SemIR::LocId loc_id,
-                          const clang::APValue& ap_value, clang::QualType type)
+                          const clang::APValue& ap_value, clang::QualType type,
+                          bool is_lvalue) -> SemIR::ConstantId;
+
+// Attempt to evaluate a C++ constexpr variable as a Carbon constant.
+auto EvalCppVarDecl(Context& context, SemIR::LocId loc_id,
+                    const clang::VarDecl* var_decl, SemIR::TypeId type_id)
     -> SemIR::ConstantId;
 
 // Attempt to evaluate a call to a C++ constexpr/consteval function as a

toolchain/check/cpp/macros.cpp

@@ -12,8 +12,6 @@
 #include "toolchain/check/cpp/constant.h"
 #include "toolchain/check/cpp/import.h"
 #include "toolchain/check/literal.h"
-#include "toolchain/check/member_access.h"
-#include "toolchain/check/type_completion.h"
 
 namespace Carbon::Check {
 
@@ -109,87 +107,17 @@ auto TryEvaluateMacro(Context& context, SemIR::LocId loc_id,
     CARBON_FATAL("failed to evaluate macro as constant expression");
   }
 
-  clang::APValue ap_value = evaluated_result.Val;
-  // TODO: Add support for other types.
-  if (result_expr->isGLValue()) {
-    const auto* value_decl =
-        ap_value.getLValueBase().get<const clang::ValueDecl*>();
-
-    if (!ap_value.hasLValuePath()) {
-      context.TODO(loc_id, "Macro expanded to lvalue with no path");
-      return SemIR::ErrorInst::InstId;
-    }
-
-    if (ap_value.isLValueOnePastTheEnd()) {
-      context.TODO(loc_id, "Macro expanded to a one-past-the-end lvalue");
-      return SemIR::ErrorInst::InstId;
-    }
-
-    auto key = SemIR::ClangDeclKey::ForNonFunctionDecl(
-        // TODO: can this const_cast be avoided?
-        const_cast<clang::ValueDecl*>(value_decl));
-
-    auto inst_id = ImportCppDecl(context, loc_id, key);
-    if (ap_value.getLValuePath().size() == 0) {
-      return inst_id;
-    }
-
-    // Import the base type so that its fields can be accessed.
-    auto var_storage = context.insts().GetAs<SemIR::VarStorage>(inst_id);
-    // TODO: currently an error isn't reachable here because incomplete
-    // array types can't be imported. Once that changes, switch to
-    // `RequireCompleteType` and handle the error.
-    CompleteTypeOrCheckFail(context, var_storage.type_id);
-
-    clang::QualType qual_type = ap_value.getLValueBase().getType();
-    for (const auto& entry : ap_value.getLValuePath()) {
-      if (qual_type->isArrayType()) {
-        context.TODO(loc_id, "Macro expanded to array type");
-      } else {
-        const auto* decl =
-            cast<clang::Decl>(entry.getAsBaseOrMember().getPointer());
-
-        const auto* field_decl = dyn_cast<clang::FieldDecl>(decl);
-        if (!field_decl) {
-          context.TODO(loc_id, "Macro expanded to a base class subobject");
-          return SemIR::ErrorInst::InstId;
-        }
-
-        auto field_inst_id =
-            ImportCppDecl(context, loc_id,
-                          SemIR::ClangDeclKey::ForNonFunctionDecl(
-                              const_cast<clang::FieldDecl*>(field_decl)));
-
-        if (field_inst_id == SemIR::ErrorInst::InstId) {
-          context.TODO(loc_id,
-                       "Unsupported field in macro expansion: " +
-                           ap_value.getAsString(context.ast_context(),
-                                                result_expr->getType()));
-          return SemIR::ErrorInst::InstId;
-        }
-
-        const SemIR::FieldDecl& field_decl_inst =
-            context.insts().GetAs<SemIR::FieldDecl>(field_inst_id);
-
-        qual_type = field_decl->getType();
-        inst_id = PerformMemberAccess(context, loc_id, inst_id,
-                                      field_decl_inst.name_id);
-      }
-    }
-
-    return inst_id;
-  } else {
-    auto const_id =
-        MapAPValueToConstant(context, loc_id, ap_value, result_expr->getType());
-    if (const_id == SemIR::ConstantId::NotConstant) {
-      context.TODO(loc_id,
-                   "Unsupported: macro evaluated to a constant of type: " +
-                       result_expr->getType().getAsString());
-      return SemIR::ErrorInst::InstId;
-    }
-
-    return context.constant_values().GetInstId(const_id);
+  auto const_id = MapAPValueToConstant(context, loc_id, evaluated_result.Val,
+                                       result_expr->getType(),
+                                       /*is_lvalue=*/result_expr->isGLValue());
+  if (const_id == SemIR::ConstantId::NotConstant) {
+    context.TODO(loc_id,
+                 "Unsupported: macro evaluated to a constant of type: " +
+                     result_expr->getType().getAsString());
+    return SemIR::ErrorInst::InstId;
   }
+
+  return context.constant_values().GetInstId(const_id);
 }
 
 }  // namespace Carbon::Check

toolchain/check/eval_inst.cpp

@@ -128,20 +128,10 @@ auto EvalConstantInst(Context& /*context*/, SemIR::ValueBinding /*inst*/)
 auto EvalConstantInst(Context& context, SemIR::InstId inst_id,
                       SemIR::AcquireValue inst) -> ConstantEvalResult {
   if (const auto* var_decl = GetAsClangVarDecl(context, inst.value_id)) {
-    // If the C++ global is constant, map it to a Carbon constant.
-    if (var_decl->isUsableInConstantExpressions(context.ast_context())) {
-      if (const auto* ap_value = var_decl->getEvaluatedValue()) {
-        auto clang_type = MapToCppType(context, inst.type_id);
-        if (clang_type.isNull()) {
-          return ConstantEvalResult::TODO;
-        }
-
-        auto const_id = MapAPValueToConstant(context, SemIR::LocId(inst_id),
-                                             *ap_value, clang_type);
-        if (const_id.has_value() && const_id.is_constant()) {
-          return ConstantEvalResult::Existing(const_id);
-        }
-      }
+    auto const_id =
+        EvalCppVarDecl(context, SemIR::LocId(inst_id), var_decl, inst.type_id);
+    if (const_id.has_value() && const_id.is_constant()) {
+      return ConstantEvalResult::Existing(const_id);
     }
 
     return ConstantEvalResult::NotConstant;

toolchain/check/testdata/interop/cpp/constexpr.carbon

@@ -46,6 +46,19 @@ class C(V:! f32) {}
 fn F() -> C(Cpp.flt);
 let x: C(123.5) = F();
 
+// --- pointer.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp inline '''
+int i = 123;
+constexpr int* _Nonnull ptr = &i;
+''';
+
+class C(V:! i32*) {}
+fn F() -> C(Cpp.ptr);
+let x: C(&Cpp.i) = F();
+
 // --- function.carbon
 
 library "[[@TEST_NAME]]";

toolchain/check/testdata/interop/cpp/macros.carbon

@@ -868,7 +868,20 @@ fn F() {
   //@dump-sem-ir-end
 }
 
-// --- fail_todo_assign_through_pointer.carbon
+// --- assign_pointer.carbon
+library "[[@TEST_NAME]]";
+
+import Cpp inline '''
+int *p;
+#define q p
+''';
+
+var n: i32;
+fn F() {
+  Cpp.q = &n;
+}
+
+// --- assign_through_pointer.carbon
 library "[[@TEST_NAME]]";
 
 import Cpp inline '''
@@ -877,18 +890,7 @@ int v = 1;
 ''';
 
 fn F() {
-  // CHECK:STDERR: fail_todo_assign_through_pointer.carbon:[[@LINE+11]]:5: error: semantics TODO: `Unsupported: macro evaluated to a constant of type: int *` [SemanticsTodo]
-  // CHECK:STDERR:   (*Cpp.m) = 2;
-  // CHECK:STDERR:     ^~~~~
-  // CHECK:STDERR: fail_todo_assign_through_pointer.carbon:[[@LINE+8]]:5: note: in `Cpp` name lookup for `m` [InCppNameLookup]
-  // CHECK:STDERR:   (*Cpp.m) = 2;
-  // CHECK:STDERR:     ^~~~~
-  // CHECK:STDERR:
-  // CHECK:STDERR: fail_todo_assign_through_pointer.carbon:[[@LINE+4]]:5: error: member name `m` not found in `Cpp` [MemberNameNotFoundInInstScope]
-  // CHECK:STDERR:   (*Cpp.m) = 2;
-  // CHECK:STDERR:     ^~~~~
-  // CHECK:STDERR:
-  (*Cpp.m) = 2;
+  *Cpp.m.Get() = 2;
 }
 
 // --- assign_subobject.carbon
@@ -932,7 +934,7 @@ fn F() {
   // CHECK:STDERR:   Cpp.m = 2;
   // CHECK:STDERR:   ^~~~~
   // CHECK:STDERR:
-  // CHECK:STDERR: fail_todo_assign_array.carbon:[[@LINE+11]]:3: error: semantics TODO: `Unsupported field in macro expansion: &s.arr[1]` [SemanticsTodo]
+  // CHECK:STDERR: fail_todo_assign_array.carbon:[[@LINE+11]]:3: error: semantics TODO: `unsupported field in lvalue path: &s.arr[1]` [SemanticsTodo]
   // CHECK:STDERR:   Cpp.m = 2;
   // CHECK:STDERR:   ^~~~~
   // CHECK:STDERR: fail_todo_assign_array.carbon:[[@LINE+8]]:3: note: in `Cpp` name lookup for `m` [InCppNameLookup]
@@ -959,7 +961,7 @@ struct B : A {} b;
 ''';
 
 fn F() {
-  // CHECK:STDERR: fail_todo_assign_base_class_member.carbon:[[@LINE+11]]:3: error: semantics TODO: `Macro expanded to a base class subobject` [SemanticsTodo]
+  // CHECK:STDERR: fail_todo_assign_base_class_member.carbon:[[@LINE+11]]:3: error: semantics TODO: `lvalue path contains a base class subobject` [SemanticsTodo]
   // CHECK:STDERR:   Cpp.m = 2;
   // CHECK:STDERR:   ^~~~~
   // CHECK:STDERR: fail_todo_assign_base_class_member.carbon:[[@LINE+8]]:3: note: in `Cpp` name lookup for `m` [InCppNameLookup]
@@ -2073,12 +2075,12 @@ fn F() {
 // CHECK:STDOUT: --- assign_subobject.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %int_32: Core.IntLiteral = int_value 32 [concrete]
+// CHECK:STDOUT:   %i32: type = class_type @Int, @Int(%int_32) [concrete]
 // CHECK:STDOUT:   %A: type = class_type @A [concrete]
 // CHECK:STDOUT:   %pattern_type.9de: type = pattern_type %A [concrete]
 // CHECK:STDOUT:   %B: type = class_type @B [concrete]
 // CHECK:STDOUT:   %A.elem: type = unbound_element_type %A, %B [concrete]
-// CHECK:STDOUT:   %int_32: Core.IntLiteral = int_value 32 [concrete]
-// CHECK:STDOUT:   %i32: type = class_type @Int, @Int(%int_32) [concrete]
 // CHECK:STDOUT:   %B.elem: type = unbound_element_type %B, %i32 [concrete]
 // CHECK:STDOUT:   %.1b7: ref %B = class_element_access imports.%a.var, element0 [concrete]
 // CHECK:STDOUT:   %.59e: ref %i32 = class_element_access %.1b7, element0 [concrete]
@@ -2101,7 +2103,7 @@ fn F() {
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
 // CHECK:STDOUT:   %Cpp: <namespace> = namespace file.%Cpp.import_cpp, [concrete] {
-// CHECK:STDOUT:     .m = @F.%.loc17_6.2
+// CHECK:STDOUT:     .m = constants.%.59e
 // CHECK:STDOUT:     import Cpp//...
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %a.patt: %pattern_type.9de = ref_binding_pattern a [concrete]
@@ -2114,11 +2116,12 @@ fn F() {
 // CHECK:STDOUT: fn @F() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   %Cpp.ref: <namespace> = name_ref Cpp, imports.%Cpp [concrete = imports.%Cpp]
+// CHECK:STDOUT:   <elided>
 // CHECK:STDOUT:   %b.ref: %A.elem = name_ref b, @A.%.1 [concrete = @A.%.1]
 // CHECK:STDOUT:   %.loc17_6.1: ref %B = class_element_access imports.%a.var, element0 [concrete = constants.%.1b7]
 // CHECK:STDOUT:   %c.ref: %B.elem = name_ref c, @B.%.1 [concrete = @B.%.1]
 // CHECK:STDOUT:   %.loc17_6.2: ref %i32 = class_element_access %.loc17_6.1, element0 [concrete = constants.%.59e]
-// CHECK:STDOUT:   %m.ref: ref %i32 = name_ref m, %.loc17_6.2 [concrete = constants.%.59e]
+// CHECK:STDOUT:   %m.ref: ref %i32 = name_ref m, constants.%.59e [concrete = constants.%.59e]
 // CHECK:STDOUT:   %int_2: Core.IntLiteral = int_value 2 [concrete = constants.%int_2.ecc]
 // CHECK:STDOUT:   %impl.elem0: %.545 = impl_witness_access constants.%ImplicitAs.impl_witness.6bc, element0 [concrete = constants.%Core.IntLiteral.as.ImplicitAs.impl.Convert.0b5]
 // CHECK:STDOUT:   %bound_method.loc17_9.1: <bound method> = bound_method %int_2, %impl.elem0 [concrete = constants.%Core.IntLiteral.as.ImplicitAs.impl.Convert.bound]

toolchain/check/testdata/interop/cpp/var/global.carbon

@@ -83,6 +83,7 @@ fn MyF() {
 // CHECK:STDOUT:   %const: type = const_type %ptr.d9e [concrete]
 // CHECK:STDOUT:   %pattern_type.8ca: type = pattern_type %const [concrete]
 // CHECK:STDOUT:   %global_ref.var: ref %ptr.d9e = var imports.%global_ref.var_patt [concrete]
+// CHECK:STDOUT:   %addr: %ptr.d9e = addr_of imports.%global.var [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -168,7 +169,8 @@ fn MyF() {
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %.loc9_37.1: ref %ptr.d9e = as_compatible %global_ref.ref [concrete = constants.%global_ref.var]
 // CHECK:STDOUT:   %.loc9_37.2: ref %ptr.d9e = converted %global_ref.ref, %.loc9_37.1 [concrete = constants.%global_ref.var]
-// CHECK:STDOUT:   %.loc9_37.3: %ptr.d9e = acquire_value %.loc9_37.2
+// CHECK:STDOUT:   %addr: %ptr.d9e = addr_of imports.%global.var [concrete = constants.%addr]
+// CHECK:STDOUT:   %.loc9_37.3: %ptr.d9e = acquire_value %.loc9_37.2 [concrete = constants.%addr]
 // CHECK:STDOUT:   %local_ref: %ptr.d9e = value_binding local_ref, %.loc9_37.3
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }