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

Code before:
      return SemIR::ErrorInst::InstId;
    }
    return type_inst_id;
  }

  context.TODO(loc_id, llvm::formatv("Unsupported: Declaration type {0}",
                                     clang_decl->getDeclKindName())
                           .str());
  return SemIR::InstId::None;
}

// Imports a `clang::NamedDecl` into Carbon and adds that name into the
// `NameScope`.
static auto ImportNameDeclIntoScope(Context& context, SemIR::LocId loc_id,
                                    SemIR::NameScopeId scope_id,
                                    SemIR::NameId name_id,
                                    clang::NamedDecl* clang_decl)
    -> SemIR::InstId {
  SemIR::InstId inst_id = SemIR::InstId::None;
  SemIR::ClangDeclStore& clang_decls = context.sem_ir().clang_decls();
  if (SemIR::ClangDeclId clang_decl_id = clang_decls.Lookup(clang_decl);
      clang_decl_id.has_value()) {
    inst_id = clang_decls.Get(clang_decl_id).inst_id;
  } else {
    inst_id = ImportNameDecl(context, loc_id, scope_id, name_id, clang_decl);
  }
  AddNameToScope(context, scope_id, name_id, inst_id);
  return inst_id;
}

auto ImportNameFromCpp(Context& context, SemIR::LocId loc_id,
                       SemIR::NameScopeId scope_id, SemIR::NameId name_id)
    -> SemIR::InstId {
  Diagnostics::AnnotationScope annotate_diagnostics(
      &context.emitter(), [&](auto& builder) {
        CARBON_DIAGNOSTIC(InCppNameLookup, Note,
                          "in `Cpp` name lookup for `{0}`", SemIR::NameId);
        builder.Note(loc_id, InCppNameLookup, name_id);
      });

  auto lookup = ClangLookup(context, scope_id, name_id);
  if (!lookup) {
    return SemIR::InstId::None;
  }

  if (!lookup->isSingleResult()) {
    context.TODO(loc_id,
                 llvm::formatv("Unsupported: Lookup succeeded but couldn't "
                               "find a single result; LookupResultKind: {0}",
                               static_cast<int>(lookup->getResultKind()))
                     .str());

Review comment (performance):
Performing the check at the top level here will mean we do the same hash table lookup multiple times in some cases. For example, for type handling we will now look up a class type twice, once here and once when we reach `MapRecordType`. Also the map lookup will never succeed for a declaration of a kind that we don't store in the map, such as a typedef or using declaration, so it's a wasted lookup in those cases.

I think due to the above, it'd be more efficient to sink the `clang_decls` lookups into the individual decl handling functions. That'd mean that instead of calling`ImportNamespaceDecl` directly in `ImportNameDecl`, we should be calling `AsCarbonNamespace`, and we should add logic to do the `clang_decls` lookup to the `ImportFunctionDecl` code path.

Code after:
struct FunctionParamsInsts {
  SemIR::InstBlockId implicit_param_patterns_id;
  SemIR::InstBlockId param_patterns_id;
  SemIR::InstId return_slot_pattern_id;
  SemIR::InstBlockId call_params_id;
};
}  // namespace

// Creates a block containing the parameter pattern instructions for the
// explicit parameters, a parameter pattern instruction for the return type and
// a block containing the call parameters of the function. Emits a callee
// pattern-match for the explicit parameter patterns and the return slot pattern
// to create the Call parameters instructions block. Currently the implicit
// parameter patterns are not taken into account. Returns the parameter patterns
// block id, the return slot pattern id, and the call parameters block id.
// Produces a diagnostic and returns `std::nullopt` if the function declaration
// has an unsupported parameter type.
static auto CreateFunctionParamsInsts(Context& context, SemIR::LocId loc_id,
                                      const clang::FunctionDecl* clang_decl)
    -> std::optional<FunctionParamsInsts> {
  if (isa<clang::CXXConstructorDecl, clang::CXXDestructorDecl>(clang_decl)) {
    context.TODO(loc_id, "Unsupported: Constructor/Destructor");
    return std::nullopt;
  }

  auto implicit_param_patterns_id =
      MakeImplicitParamPatternsBlockId(context, loc_id, *clang_decl);
  if (!implicit_param_patterns_id.has_value()) {
    return std::nullopt;
  }
  auto param_patterns_id =
      MakeParamPatternsBlockId(context, loc_id, *clang_decl);
  if (!param_patterns_id.has_value()) {
    return std::nullopt;
  }
  auto return_slot_pattern_id = GetReturnType(context, loc_id, clang_decl);
  if (SemIR::ErrorInst::InstId == return_slot_pattern_id) {
    return std::nullopt;
  }

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

  return {{.implicit_param_patterns_id = implicit_param_patterns_id,
           .param_patterns_id = param_patterns_id,
           .return_slot_pattern_id = return_slot_pattern_id,
           .call_params_id = call_params_id}};
}

// Imports a function declaration from Clang to Carbon. If successful, returns