From 943cd419240a87b207122559729a6dfbc8494dfb Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Mon, 16 Mar 2026 15:19:20 -0400 Subject: [PATCH] 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 | 133 +++++++++++++++++- toolchain/check/cpp/constant.h | 7 +- toolchain/check/cpp/macros.cpp | 92 ++---------- toolchain/check/eval_inst.cpp | 18 +-- .../testdata/interop/cpp/constexpr.carbon | 13 ++ .../check/testdata/interop/cpp/macros.carbon | 41 +++--- .../testdata/interop/cpp/var/global.carbon | 4 +- 7 files changed, 185 insertions(+), 123 deletions(-) diff --git a/toolchain/check/cpp/constant.cpp b/toolchain/check/cpp/constant.cpp index 1c71c912c2ae..e90350c391de 100644 --- a/toolchain/check/cpp/constant.cpp +++ b/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(); + + 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(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(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(entry.getAsBaseOrMember().getPointer()); + + const auto* field_decl = dyn_cast(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(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(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( + 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 diff --git a/toolchain/check/cpp/constant.h b/toolchain/check/cpp/constant.h index 8b44e5ac30cb..89a7fc8b8e32 100644 --- a/toolchain/check/cpp/constant.h +++ b/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 diff --git a/toolchain/check/cpp/macros.cpp b/toolchain/check/cpp/macros.cpp index 129bece9f1d8..91b0570878ba 100644 --- a/toolchain/check/cpp/macros.cpp +++ b/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(); - - 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(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(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(entry.getAsBaseOrMember().getPointer()); - - const auto* field_decl = dyn_cast(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(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(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 diff --git a/toolchain/check/eval_inst.cpp b/toolchain/check/eval_inst.cpp index d2a465349a73..d5c801caba48 100644 --- a/toolchain/check/eval_inst.cpp +++ b/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; diff --git a/toolchain/check/testdata/interop/cpp/constexpr.carbon b/toolchain/check/testdata/interop/cpp/constexpr.carbon index a5b63dd6eb2a..cfa3d7ed78a9 100644 --- a/toolchain/check/testdata/interop/cpp/constexpr.carbon +++ b/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]]"; diff --git a/toolchain/check/testdata/interop/cpp/macros.carbon b/toolchain/check/testdata/interop/cpp/macros.carbon index 505cbce131a7..b363b5bdb80f 100644 --- a/toolchain/check/testdata/interop/cpp/macros.carbon +++ b/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 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: = name_ref Cpp, imports.%Cpp [concrete = imports.%Cpp] +// CHECK:STDOUT: // 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 %int_2, %impl.elem0 [concrete = constants.%Core.IntLiteral.as.ImplicitAs.impl.Convert.bound] diff --git a/toolchain/check/testdata/interop/cpp/var/global.carbon b/toolchain/check/testdata/interop/cpp/var/global.carbon index 2260667db90d..48ff08f56bd0 100644 --- a/toolchain/check/testdata/interop/cpp/var/global.carbon +++ b/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: }