Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions toolchain/check/import_cpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2211,7 +2211,8 @@ static auto GetClangOperatorKind(Context& context, SemIR::LocId loc_id,
return std::nullopt;
}

auto ImportOperatorFromCpp(Context& context, SemIR::LocId loc_id, Operator op)
auto ImportOperatorFromCpp(Context& context, SemIR::LocId loc_id,
SemIR::NameScopeId scope_id, Operator op)
-> SemIR::ScopeLookupResult {
Diagnostics::AnnotationScope annotate_diagnostics(
&context.emitter(), [&](auto& builder) {
Expand All @@ -2231,7 +2232,7 @@ auto ImportOperatorFromCpp(Context& context, SemIR::LocId loc_id, Operator op)
// into C++ types. See
// https://github.com/carbon-language/carbon-lang/pull/5996/files/5d01fa69511b76f87efbc0387f5e40abcf4c911a#r2316950123
auto decl_and_access = ClangLookupDeclarationName(
context, loc_id, SemIR::NameScopeId::None,
context, loc_id, scope_id,
context.ast_context().DeclarationNames.getCXXOperatorName(*op_kind));

if (!decl_and_access) {
Expand Down
3 changes: 2 additions & 1 deletion toolchain/check/import_cpp.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ auto ImportNameFromCpp(Context& context, SemIR::LocId loc_id,

// Looks up the given operator in the Clang AST generated when importing C++
// code and returns a lookup result.
auto ImportOperatorFromCpp(Context& context, SemIR::LocId loc_id, Operator op)
auto ImportOperatorFromCpp(Context& context, SemIR::LocId loc_id,
SemIR::NameScopeId scope_id, Operator op)
-> SemIR::ScopeLookupResult;

// Given a Carbon class declaration that was imported from some kind of C++
Expand Down
54 changes: 43 additions & 11 deletions toolchain/check/operator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@

#include "toolchain/check/operator.h"

#include <optional>

#include "toolchain/check/call.h"
#include "toolchain/check/context.h"
#include "toolchain/check/generic.h"
#include "toolchain/check/import_cpp.h"
#include "toolchain/check/member_access.h"
#include "toolchain/check/name_lookup.h"
#include "toolchain/sem_ir/class.h"
#include "toolchain/sem_ir/ids.h"
#include "toolchain/sem_ir/name_scope.h"
#include "toolchain/sem_ir/typed_insts.h"

namespace Carbon::Check {
Expand All @@ -35,21 +39,35 @@ static auto GetOperatorOpFunction(Context& context, SemIR::LocId loc_id,
op_name_id);
}

// Returns whether the type of the instruction is a C++ class.
static auto IsOfCppClassType(Context& context, SemIR::InstId inst_id) -> bool {
// If the instruction is a C++ class, returns its parent scope id. Otherwise
// returns `std::nullopt`.
static auto GetCppClassTypeParentScope(Context& context, SemIR::InstId inst_id)
-> std::optional<SemIR::NameScopeId> {
auto class_type = context.insts().TryGetAs<SemIR::ClassType>(
context.types().GetInstId(context.insts().Get(inst_id).type_id()));
if (!class_type) {
// Not a class.
return false;
return std::nullopt;
}

const auto& class_info = context.classes().Get(class_type->class_id);
if (!class_info.is_complete()) {
return false;
const SemIR::Class& class_info = context.classes().Get(class_type->class_id);
if (!class_info.is_complete() ||
!context.name_scopes().Get(class_info.scope_id).is_cpp_scope()) {
// Not a C++ class.
return std::nullopt;
}

return context.name_scopes().Get(class_info.scope_id).is_cpp_scope();
SemIR::NameScopeId parent_scope_id = class_info.parent_scope_id;
do {
SemIR::NameScope& scope = context.name_scopes().Get(parent_scope_id);
if (context.insts().Is<SemIR::Namespace>(scope.inst_id())) {
break;
}
parent_scope_id = scope.parent_scope_id();

} while (parent_scope_id.has_value());

return parent_scope_id;
}

auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
Expand All @@ -60,9 +78,13 @@ auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
// the C++ operator.
// TODO: Change impl lookup instead. See
// https://github.com/carbon-language/carbon-lang/blob/db0a00d713015436844c55e7ac190a0f95556499/toolchain/check/operator.cpp#L76
if (IsOfCppClassType(context, operand_id)) {
// TODO: We should do ADL-only lookup for operators
// (`Sema::ArgumentDependentLookup`), when we support mapping Carbon types
// into C++ types.
auto cpp_parent_scope_id = GetCppClassTypeParentScope(context, operand_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call Sema::ArgumentDependentLookup here instead of reimplementing something similar? I think we'd need #5891 to land first, so that we have expressions corresponding to the arguments at the point where we do the lookup.

If you'd prefer to do it this way for the time being, I think that's fine, but please add a TODO to replace this with a call to ArgumentDependentLookup when we can.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, waiting for #5891.
Added TODOs for now (similar to the one in import_cpp.cpp.

if (cpp_parent_scope_id) {
SemIR::ScopeLookupResult cpp_lookup_result =
ImportOperatorFromCpp(context, loc_id, op);
ImportOperatorFromCpp(context, loc_id, *cpp_parent_scope_id, op);
if (cpp_lookup_result.is_found()) {
return PerformCall(context, loc_id, cpp_lookup_result.target_inst_id(),
{operand_id});
Expand Down Expand Up @@ -95,9 +117,19 @@ auto BuildBinaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
// https://github.com/carbon-language/carbon-lang/pull/5996/files/5d01fa69511b76f87efbc0387f5e40abcf4c911a#r2308666348
// and
// https://github.com/carbon-language/carbon-lang/pull/5996/files/5d01fa69511b76f87efbc0387f5e40abcf4c911a#r2308664536
if (IsOfCppClassType(context, lhs_id) || IsOfCppClassType(context, rhs_id)) {
// TODO: We should do ADL-only lookup for operators
// (`Sema::ArgumentDependentLookup`), when we support mapping Carbon types
// into C++ types.
llvm::SmallVector<SemIR::NameScopeId, 2> cpp_operand_parent_scope_ids;
for (SemIR::InstId operand_id : {lhs_id, rhs_id}) {
auto cpp_parent_scope_id = GetCppClassTypeParentScope(context, operand_id);
if (!cpp_parent_scope_id || llvm::is_contained(cpp_operand_parent_scope_ids,
*cpp_parent_scope_id)) {
continue;
}
cpp_operand_parent_scope_ids.push_back(*cpp_parent_scope_id);
SemIR::ScopeLookupResult cpp_lookup_result =
ImportOperatorFromCpp(context, loc_id, op);
ImportOperatorFromCpp(context, loc_id, *cpp_parent_scope_id, op);
if (cpp_lookup_result.is_found()) {
return PerformCall(context, loc_id, cpp_lookup_result.target_inst_id(),
{lhs_id, rhs_id});
Expand Down
Loading
Loading