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

Code before:
static auto CreateFunctionSignatureInsts(Context& context, SemIR::LocId loc_id,
                                         clang::FunctionDecl* clang_decl,
                                         int num_params)
    -> std::optional<FunctionSignatureInsts> {
  context.full_pattern_stack().PushFullPattern(
      FullPatternStack::Kind::ImplicitParamList);
  auto pop = llvm::make_scope_exit(
      [&context] { context.full_pattern_stack().PopFullPattern(); });
  auto implicit_param_patterns_id =
      MakeImplicitParamPatternsBlockId(context, loc_id, *clang_decl);
  if (!implicit_param_patterns_id.has_value()) {
    return std::nullopt;
  }
  context.full_pattern_stack().EndImplicitParamList();
  auto param_patterns_id =
      MakeParamPatternsBlockId(context, loc_id, *clang_decl, num_params);
  if (!param_patterns_id.has_value()) {
    return std::nullopt;
  }
  auto [return_type_inst_id, return_patterns_id] =
      GetReturnInfo(context, loc_id, clang_decl);
  if (return_type_inst_id == SemIR::ErrorInst::TypeInstId) {
    return std::nullopt;
  }
  pop.release();
  context.full_pattern_stack().PopFullPattern();

  auto call_params_id =
      CalleePatternMatch(context, implicit_param_patterns_id, param_patterns_id,
                         return_patterns_id);

  return {{.implicit_param_patterns_id = implicit_param_patterns_id,
           .param_patterns_id = param_patterns_id,
           .return_type_inst_id = return_type_inst_id,
           .return_patterns_id = return_patterns_id,
           .call_params_id = call_params_id}};
}

// Returns the Carbon function name for the given function.
static auto GetFunctionName(Context& context, clang::FunctionDecl* clang_decl)
    -> SemIR::NameId {
  switch (clang_decl->getDeclName().getNameKind()) {
    case clang::DeclarationName::CXXConstructorName: {
      auto key = SemIR::ClangDeclKey(
          cast<clang::CXXConstructorDecl>(clang_decl)->getParent());
      return context.classes()
          .Get(context.insts()
                   .GetAs<SemIR::ClassDecl>(LookupClangDeclInstId(context, key))
                   .class_id)
          .name_id;
    }

Review comment (suggestion):
rather than popping then doing the action anyway - could you introduce an explicit scope that ends here to do the work? I'm open to disagreement on that, though - explicit scopes are a bit quirky to read, but also dismissing a scoped action then doing the action anyway at a distance feels a bit unclear.

Code after:
static auto CreateFunctionSignatureInsts(Context& context, SemIR::LocId loc_id,
                                         clang::FunctionDecl* clang_decl,
                                         int num_params)
    -> std::optional<FunctionSignatureInsts> {
  context.full_pattern_stack().PushFullPattern(
      FullPatternStack::Kind::ImplicitParamList);
  std::optional pop = llvm::make_scope_exit(
      [&context] { context.full_pattern_stack().PopFullPattern(); });
  auto implicit_param_patterns_id =
      MakeImplicitParamPatternsBlockId(context, loc_id, *clang_decl);
  if (!implicit_param_patterns_id.has_value()) {
    return std::nullopt;
  }
  context.full_pattern_stack().EndImplicitParamList();
  auto param_patterns_id =
      MakeParamPatternsBlockId(context, loc_id, *clang_decl, num_params);
  if (!param_patterns_id.has_value()) {
    return std::nullopt;
  }
  auto [return_type_inst_id, return_patterns_id] =
      GetReturnInfo(context, loc_id, clang_decl);
  if (return_type_inst_id == SemIR::ErrorInst::TypeInstId) {
    return std::nullopt;
  }
  pop.reset();

  auto call_params_id =
      CalleePatternMatch(context, implicit_param_patterns_id, param_patterns_id,
                         return_patterns_id);

  return {{.implicit_param_patterns_id = implicit_param_patterns_id,
           .param_patterns_id = param_patterns_id,
           .return_type_inst_id = return_type_inst_id,
           .return_patterns_id = return_patterns_id,
           .call_params_id = call_params_id}};
}

// Returns the Carbon function name for the given function.
static auto GetFunctionName(Context& context, clang::FunctionDecl* clang_decl)
    -> SemIR::NameId {
  switch (clang_decl->getDeclName().getNameKind()) {
    case clang::DeclarationName::CXXConstructorName: {
      auto key = SemIR::ClangDeclKey(
          cast<clang::CXXConstructorDecl>(clang_decl)->getParent());
      return context.classes()
          .Get(context.insts()
                   .GetAs<SemIR::ClassDecl>(LookupClangDeclInstId(context, key))
                   .class_id)
          .name_id;
    }