diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index 3e997c675cdf..3c8c896158c7 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -1520,28 +1520,19 @@ static auto MapParameterType( .kind = GetParamPatternKindForPassingMode(passing_mode)}; } -// Returns a block containing the `self` parameter pattern for the given -// function declaration, or `Empty` if it is not a method. -// Returns `None` on error. -static auto MakeSelfParamPatternBlockId( +// Creates a Carbon parameter pattern for a C++ parameter with the given type, +// passing mode, and parameter name. Returns the IDs of the pattern inst and +// the inst representing the parameter type. +static auto MakeParamPattern( Context& context, SemIR::LocId loc_id, - SemIR::ImportIRInstId import_ir_inst_id, - const clang::FunctionDecl& clang_decl, - SemIR::ClangDeclSignatureId signature_id) -> SemIR::InstBlockId { - if (!IsObjectMemberFunction(clang_decl)) { - return SemIR::InstBlockId::Empty; - } - const auto* method_decl = cast(&clang_decl); - - // Build a `self` parameter from the object parameter. + SemIR::ImportIRInstId import_ir_inst_id, clang::QualType type, + SemIR::ClangDeclSignature::PassingMode passing_mode, SemIR::NameId name_id) + -> std::pair { + // Mark the start of a region of insts, needed for the type expression + // created later with the call of `ConsumeExprRegionForPattern()`. BeginExprRegionForPattern(context); - clang::QualType param_type = - method_decl->getFunctionObjectParameterReferenceType(); - const auto& signature = context.clang_decl_signatures().Get(signature_id); - SemIR::ClangDeclSignature::PassingMode passing_mode = - signature.self_passing_mode; - auto param_info = MapParameterType(context, loc_id, param_type, passing_mode); + auto param_info = MapParameterType(context, loc_id, type, passing_mode); auto [type_inst_id, type_id] = param_info.type; SemIR::ExprRegionId type_expr_region_id = ConsumeExprRegionForPattern(context, type_inst_id); @@ -1549,19 +1540,14 @@ static auto MakeSelfParamPatternBlockId( EndEmptyExprRegionForPattern(context); if (!type_id.has_value()) { - context.TODO(loc_id, - llvm::formatv("Unsupported: object parameter type: {0}", - param_type.getAsString())); - return SemIR::InstBlockId::None; + context.TODO(loc_id, llvm::formatv("Unsupported: parameter type: {0}", + type.getAsString())); + return {SemIR::ErrorInst::InstId, SemIR::ErrorInst::TypeInstId}; } - // TODO: Use a location associated with the object parameter instead of the - // location of the function as a whole. - auto pattern_id = - AddParamPattern(context, import_ir_inst_id, SemIR::NameId::SelfValue, - type_expr_region_id, type_id, param_info.kind); - - return context.inst_blocks().Add({pattern_id}); + return {AddParamPattern(context, import_ir_inst_id, name_id, + type_expr_region_id, type_id, param_info.kind), + type_inst_id}; } // Returns a block id for the explicit parameters of the given function @@ -1576,11 +1562,33 @@ static auto MakeParamPatternsBlockId(Context& context, SemIR::LocId loc_id, const clang::FunctionDecl& clang_decl, SemIR::ClangDeclSignatureId signature_id) -> SemIR::InstBlockId { + // The `self` parameter of a method is the first entry in the explicit + // parameter list. Build it (if any) first, then the remaining explicit + // parameters. + bool has_self_param = IsObjectMemberFunction(clang_decl); const auto& signature = context.clang_decl_signatures().Get(signature_id); llvm::SmallVector param_ids; llvm::SmallVector param_type_ids; - param_ids.reserve(signature.num_params); + param_ids.reserve(signature.num_params + has_self_param); param_type_ids.reserve(signature.num_params); + if (has_self_param) { + const auto* method_decl = cast(&clang_decl); + clang::QualType param_type = + method_decl->getFunctionObjectParameterReferenceType(); + const auto& signature = context.clang_decl_signatures().Get(signature_id); + + // TODO: Use a location associated with the object parameter instead of the + // location of the function as a whole. + auto [self_param_pattern_id, _] = + MakeParamPattern(context, loc_id, import_ir_inst_id, param_type, + signature.self_passing_mode, SemIR::NameId::SelfValue); + if (self_param_pattern_id == SemIR::ErrorInst::InstId) { + return SemIR::InstBlockId::None; + } + param_ids.push_back(self_param_pattern_id); + // We don't push to param_type_ids because the self parameter can't be part + // of a parameter tuple, which is the only case where we use param_type_ids. + } CARBON_CHECK(static_cast(clang_decl.getNumNonObjectParams()) >= signature.num_params, "Function has fewer parameters than requested: {0} < {1}", @@ -1598,25 +1606,6 @@ static auto MakeParamPatternsBlockId(Context& context, SemIR::LocId loc_id, clang::QualType param_type = ClangGetUnqualifiedTypePreserveNonNull(context, orig_param_type); - // Mark the start of a region of insts, needed for the type expression - // created later with the call of `ConsumeExprRegionForPattern()`. - BeginExprRegionForPattern(context); - auto param_info = MapParameterType(context, loc_id, param_type, - signature.GetPassingMode(i)); - auto [type_inst_id, type_id] = param_info.type; - // Type expression of the binding pattern - a single-entry/single-exit - // region that allows control flow in the type expression e.g. fn F(x: if C - // then i32 else i64). - SemIR::ExprRegionId type_expr_region_id = - ConsumeExprRegionForPattern(context, type_inst_id); - EndEmptyExprRegionForPattern(context); - - if (!type_id.has_value()) { - context.TODO(loc_id, llvm::formatv("Unsupported: parameter type: {0}", - orig_param_type.getAsString())); - return SemIR::InstBlockId::None; - } - llvm::StringRef param_name = param->getName(); SemIR::NameId name_id = param_name.empty() @@ -1625,13 +1614,16 @@ static auto MakeParamPatternsBlockId(Context& context, SemIR::LocId loc_id, ? SemIR::NameId::Underscore : AddIdentifierName(context, param_name); - SemIR::LocId param_loc_id = + SemIR::ImportIRInstId param_loc_id = AddImportIRInst(context.sem_ir(), param->getLocation()); // TODO: Add template support. - SemIR::InstId pattern_id = - AddParamPattern(context, param_loc_id, name_id, type_expr_region_id, - type_id, param_info.kind); + auto [pattern_id, type_inst_id] = + MakeParamPattern(context, loc_id, param_loc_id, param_type, + signature.GetPassingMode(i), name_id); + if (pattern_id == SemIR::ErrorInst::InstId) { + return SemIR::InstBlockId::None; + } param_ids.push_back(pattern_id); param_type_ids.push_back(type_inst_id); } @@ -1645,6 +1637,7 @@ static auto MakeParamPatternsBlockId(Context& context, SemIR::LocId loc_id, case SemIR::ClangDeclSignature::TuplePattern: { // Replace the parameters with a single tuple pattern containing the // converted parameter list. + CARBON_CHECK(!has_self_param); auto param_block_id = context.inst_blocks().Add(param_ids); auto tuple_pattern_type_id = GetPatternType(context, GetTupleType(context, param_type_ids)); @@ -1808,33 +1801,14 @@ static auto CreateFunctionSignatureInsts( SemIR::ImportIRInstId import_ir_inst_id, clang::FunctionDecl* clang_decl, SemIR::ClangDeclSignatureId signature_id) -> std::optional { - // The `self` parameter of a method (proposal #7016) is the - // first entry in the explicit parameter list. Build it (if any) first, then - // the remaining explicit parameters, and concatenate them into a single - // explicit parameter list. context.full_pattern_stack().StartExplicitParamList(); - auto self_param_patterns_id = MakeSelfParamPatternBlockId( + auto param_patterns_id = MakeParamPatternsBlockId( context, loc_id, import_ir_inst_id, *clang_decl, signature_id); - if (!self_param_patterns_id.has_value()) { - return std::nullopt; - } - auto explicit_param_patterns_id = MakeParamPatternsBlockId( - context, loc_id, import_ir_inst_id, *clang_decl, signature_id); - if (!explicit_param_patterns_id.has_value()) { + if (!param_patterns_id.has_value()) { return std::nullopt; } context.full_pattern_stack().EndExplicitParamList(); - auto self_param_patterns = - context.inst_blocks().GetOrEmpty(self_param_patterns_id); - auto param_patterns_id = explicit_param_patterns_id; - if (!self_param_patterns.empty()) { - llvm::SmallVector combined(self_param_patterns); - llvm::append_range( - combined, context.inst_blocks().GetOrEmpty(explicit_param_patterns_id)); - param_patterns_id = context.inst_blocks().Add(combined); - } - auto [return_type_inst_id, return_form_inst_id, return_pattern_id] = GetReturnInfo(context, loc_id, clang_decl); if (return_type_inst_id == SemIR::ErrorInst::TypeInstId) { diff --git a/toolchain/check/testdata/interop/cpp/class/import/method.carbon b/toolchain/check/testdata/interop/cpp/class/import/method.carbon index 29abd35bcf3a..622dbafc52af 100644 --- a/toolchain/check/testdata/interop/cpp/class/import/method.carbon +++ b/toolchain/check/testdata/interop/cpp/class/import/method.carbon @@ -109,7 +109,7 @@ import Cpp library "object_param_qualifiers.h"; fn Ref(p: Cpp.HasQualifiers*) { // TODO: This should eventually be accepted if we support `volatile`. - // CHECK:STDERR: fail_todo_bad_object_param_qualifiers_by_ref.carbon:[[@LINE+4]]:3: error: semantics TODO: `Unsupported: object parameter type: volatile struct HasQualifiers &` [SemanticsTodo] + // CHECK:STDERR: fail_todo_bad_object_param_qualifiers_by_ref.carbon:[[@LINE+4]]:3: error: semantics TODO: `Unsupported: parameter type: volatile struct HasQualifiers &` [SemanticsTodo] // CHECK:STDERR: p->volatile_this(); // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~ // CHECK:STDERR: