If a name is not found in a class, perform lookup into base classes. (c…

…arbon-language#3502)

This builds out a little infrastructure for one name scope to `extend`
another. We'll need more refinement here to cover other cases, but this
should provide some foundation for that future work.

---------

Co-authored-by: Jon Ross-Perkins <[email protected]>

toolchain/check/check.cpp

@@ -87,7 +87,7 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
  if (self_import != unit_info.package_imports_map.end()) {
  auto& package_scope =
  context.name_scopes().Get(SemIR::NameScopeId::Package);
- package_scope.has_load_error = self_import->second.has_load_error;
+ package_scope.has_error = self_import->second.has_load_error;
 
  for (const auto& import : self_import->second.imports) {
  const auto& import_sem_ir = **import.unit_info->unit->sem_ir;
@@ -96,7 +96,7 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
 
  // If an import of the current package caused an error for the imported
  // file, it transitively affects the current file too.
- package_scope.has_load_error |= import_scope.has_load_error;
+ package_scope.has_error |= import_scope.has_error;
 
  auto ir_id = context.sem_ir().cross_ref_irs().Add(&import_sem_ir);
 
@@ -124,7 +124,7 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
 
  // Push the scope.
  context.PushScope(package_inst, SemIR::NameScopeId::Package,
- package_scope.has_load_error);
+ package_scope.has_error);
  } else {
  // Push the scope; there are no names to add.
  context.PushScope(package_inst, SemIR::NameScopeId::Package);

toolchain/check/context.cpp

@@ -205,7 +205,8 @@ auto Context::ResolveIfLazyImportRef(SemIR::InstId inst_id) -> void {
  }
 }
 
-auto Context::LookupNameInDecl(Parse::NodeId parse_node, SemIR::NameId name_id,
+auto Context::LookupNameInDecl(Parse::NodeId /*parse_node*/,
+ SemIR::NameId name_id,
  SemIR::NameScopeId scope_id) -> SemIR::InstId {
  if (scope_id == SemIR::NameScopeId::Invalid) {
  // Look for a name in the current scope only. There are two cases where the
@@ -243,10 +244,16 @@ auto Context::LookupNameInDecl(Parse::NodeId parse_node, SemIR::NameId name_id,
  }
  return SemIR::InstId::Invalid;
  } else {
- // TODO: Once we support `extend`, do not look into `extend`ed scopes here,
- // following the same logic as above.
- return LookupQualifiedName(parse_node, name_id, scope_id,
- /*required=*/false);
+ // We do not look into `extend`ed scopes here. A qualified name in a
+ // declaration must specify the exact scope in which the name was originally
+ // introduced:
+ //
+ // base class A { fn F(); }
+ // class B { extend base: A; }
+ //
+ // // Error, no `F` in `B`.
+ // fn B.F() {}
+ return LookupNameInExactScope(name_id, name_scopes().Get(scope_id));
  }
 }
 
@@ -296,26 +303,63 @@ auto Context::LookupUnqualifiedName(Parse::NodeId parse_node,
  return SemIR::InstId::BuiltinError;
 }
 
-auto Context::LookupQualifiedName(Parse::NodeId parse_node,
- SemIR::NameId name_id,
- SemIR::NameScopeId scope_id, bool required)
+auto Context::LookupNameInExactScope(SemIR::NameId name_id,
+ const SemIR::NameScope& scope)
  -> SemIR::InstId {
- CARBON_CHECK(scope_id.is_valid()) << "No scope to perform lookup into";
- const auto& scope = name_scopes().Get(scope_id);
  if (auto it = scope.names.find(name_id); it != scope.names.end()) {
  ResolveIfLazyImportRef(it->second);
  return it->second;
  }
+ return SemIR::InstId::Invalid;
+}
+
+auto Context::LookupQualifiedName(Parse::NodeId parse_node,
+ SemIR::NameId name_id,
+ SemIR::NameScopeId scope_id, bool required)
+ -> SemIR::InstId {
+ llvm::SmallVector<SemIR::NameScopeId> scope_ids = {scope_id};
+ auto result_id = SemIR::InstId::Invalid;
+ bool has_error = false;
+
+ // Walk this scope and, if nothing is found here, the scopes it extends.
+ while (!scope_ids.empty()) {
+ const auto& scope = name_scopes().Get(scope_ids.pop_back_val());
+ has_error |= scope.has_error;
+
+ auto scope_result_id = LookupNameInExactScope(name_id, scope);
+ if (!scope_result_id.is_valid()) {
+ // Nothing found in this scope: also look in its extended scopes.
+ auto extended = llvm::reverse(scope.extended_scopes);
+ scope_ids.append(extended.begin(), extended.end());
+ continue;
+ }
 
- // TODO: Also perform lookups into `extend`ed scopes.
+ // If this is our second lookup result, diagnose an ambiguity.
+ if (result_id.is_valid()) {
+ // TODO: This is currently not reachable because the only scope that can
+ // extend is a class scope, and it can only extend a single base class.
+ // Add test coverage once this is possible.
+ CARBON_DIAGNOSTIC(
+ NameAmbiguousDueToExtend, Error,
+ "Ambiguous use of name `{0}` found in multiple extended scopes.",
+ std::string);
+ emitter_->Emit(parse_node, NameAmbiguousDueToExtend,
+ names().GetFormatted(name_id).str());
+ // TODO: Add notes pointing to the scopes.
+ return SemIR::InstId::BuiltinError;
+ }
 
- if (!required) {
- return SemIR::InstId::Invalid;
+ result_id = scope_result_id;
  }
- if (!scope.has_load_error) {
- DiagnoseNameNotFound(parse_node, name_id);
+
+ if (required && !result_id.is_valid()) {
+ if (!has_error) {
+ DiagnoseNameNotFound(parse_node, name_id);
+ }
+ return SemIR::InstId::BuiltinError;
  }
- return SemIR::InstId::BuiltinError;
+
+ return result_id;
 }
 
 auto Context::PushScope(SemIR::InstId scope_inst_id,

toolchain/check/context.h

@@ -85,6 +85,12 @@ class Context {
  auto LookupUnqualifiedName(Parse::NodeId parse_node, SemIR::NameId name_id)
  -> SemIR::InstId;
 
+ // Performs a name lookup in a specified scope, returning the referenced
+ // instruction. Does not look into extended scopes. Returns an invalid
+ // instruction if the name is not found.
+ auto LookupNameInExactScope(SemIR::NameId name_id,
+ const SemIR::NameScope& scope) -> SemIR::InstId;
+
  // Performs a qualified name lookup in a specified scope and in scopes that
  // it extends, returning the referenced instruction.
  auto LookupQualifiedName(Parse::NodeId parse_node, SemIR::NameId name_id,

toolchain/check/convert.cpp

@@ -1032,12 +1032,19 @@ auto ConvertToValueOrRefExpr(Context& context, SemIR::InstId expr_id)
 }
 
 auto ConvertToValueOfType(Context& context, Parse::NodeId parse_node,
- SemIR::InstId value_id, SemIR::TypeId type_id)
+ SemIR::InstId expr_id, SemIR::TypeId type_id)
  -> SemIR::InstId {
- return Convert(context, parse_node, value_id,
+ return Convert(context, parse_node, expr_id,
  {.kind = ConversionTarget::Value, .type_id = type_id});
 }
 
+auto ConvertToValueOrRefOfType(Context& context, Parse::NodeId parse_node,
+ SemIR::InstId expr_id, SemIR::TypeId type_id)
+ -> SemIR::InstId {
+ return Convert(context, parse_node, expr_id,
+ {.kind = ConversionTarget::ValueOrRef, .type_id = type_id});
+}
+
 auto ConvertToBoolValue(Context& context, Parse::NodeId parse_node,
  SemIR::InstId value_id) -> SemIR::InstId {
  return ConvertToValueOfType(

toolchain/check/convert.h

@@ -70,9 +70,15 @@ auto ConvertToValueExpr(Context& context, SemIR::InstId expr_id)
 auto ConvertToValueOrRefExpr(Context& context, SemIR::InstId expr_id)
  -> SemIR::InstId;
 
-// Converts `value_id` to a value expression of type `type_id`.
+// Converts `expr_id` to a value expression of type `type_id`.
 auto ConvertToValueOfType(Context& context, Parse::NodeId parse_node,
- SemIR::InstId value_id, SemIR::TypeId type_id)
+ SemIR::InstId expr_id, SemIR::TypeId type_id)
+ -> SemIR::InstId;
+
+// Convert the given expression to a value or reference expression of the given
+// type.
+auto ConvertToValueOrRefOfType(Context& context, Parse::NodeId parse_node,
+ SemIR::InstId expr_id, SemIR::TypeId type_id)
  -> SemIR::InstId;
 
 // Converts `value_id` to a value expression of type `bool`.

toolchain/check/decl_name_stack.cpp

@@ -146,7 +146,7 @@ auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context) -> void {
  name_context.state = NameContext::State::Resolved;
  name_context.target_scope_id = scope_id;
  context_->PushScope(name_context.resolved_inst_id, scope_id,
- context_->name_scopes().Get(scope_id).has_load_error);
+ context_->name_scopes().Get(scope_id).has_error);
  break;
  }
  default:

toolchain/check/handle_class.cpp

@@ -174,6 +174,77 @@ auto HandleBaseColon(Context& /*context*/, Parse::NodeId /*parse_node*/)
  return true;
 }
 
+namespace {
+// Information gathered about a base type specified in a `base` declaration.
+struct BaseInfo {
+ // A `BaseInfo` representing an erroneous base.
+ static const BaseInfo Error;
+
+ SemIR::TypeId type_id;
+ SemIR::NameScopeId scope_id;
+};
+constexpr BaseInfo BaseInfo::Error = {.type_id = SemIR::TypeId::Error,
+ .scope_id = SemIR::NameScopeId::Invalid};
+} // namespace
+
+// If `type_id` is a class type, get its corresponding `SemIR::Class` object.
+// Otherwise returns `nullptr`.
+static auto TryGetAsClass(Context& context, SemIR::TypeId type_id)
+ -> SemIR::Class* {
+ auto class_type = context.types().TryGetAs<SemIR::ClassType>(type_id);
+ if (!class_type) {
+ return nullptr;
+ }
+ return &context.classes().Get(class_type->class_id);
+}
+
+// Diagnoses an attempt to derive from a final type.
+static auto DiagnoseBaseIsFinal(Context& context, Parse::NodeId parse_node,
+ SemIR::TypeId base_type_id) -> void {
+ CARBON_DIAGNOSTIC(BaseIsFinal, Error,
+ "Deriving from final type `{0}`. Base type must be an "
+ "`abstract` or `base` class.",
+ std::string);
+ context.emitter().Emit(parse_node, BaseIsFinal,
+ context.sem_ir().StringifyType(base_type_id));
+}
+
+// Checks that the specified base type is valid.
+static auto CheckBaseType(Context& context, Parse::NodeId parse_node,
+ SemIR::InstId base_expr_id) -> BaseInfo {
+ auto base_type_id = ExprAsType(context, parse_node, base_expr_id);
+ base_type_id = context.AsCompleteType(base_type_id, [&] {
+ CARBON_DIAGNOSTIC(IncompleteTypeInBaseDecl, Error,
+ "Base `{0}` is an incomplete type.", std::string);
+ return context.emitter().Build(
+ parse_node, IncompleteTypeInBaseDecl,
+ context.sem_ir().StringifyType(base_type_id));
+ });
+
+ if (base_type_id == SemIR::TypeId::Error) {
+ return BaseInfo::Error;
+ }
+
+ auto* base_class_info = TryGetAsClass(context, base_type_id);
+
+ // The base must not be a final class.
+ if (!base_class_info) {
+ // For now, we treat all types that aren't introduced by a `class`
+ // declaration as being final classes.
+ // TODO: Once we have a better idea of which types are considered to be
+ // classes, produce a better diagnostic for deriving from a non-class type.
+ DiagnoseBaseIsFinal(context, parse_node, base_type_id);
+ return BaseInfo::Error;
+ }
+ if (base_class_info->inheritance_kind == SemIR::Class::Final) {
+ DiagnoseBaseIsFinal(context, parse_node, base_type_id);
+ }
+
+ CARBON_CHECK(base_class_info->scope_id.is_valid())
+ << "Complete class should have a scope";
+ return {.type_id = base_type_id, .scope_id = base_class_info->scope_id};
+}
+
 auto HandleBaseDecl(Context& context, Parse::NodeId parse_node) -> bool {
  auto base_type_expr_id = context.node_stack().PopExpr();
 
@@ -211,54 +282,39 @@ auto HandleBaseDecl(Context& context, Parse::NodeId parse_node) -> bool {
  return true;
  }
 
- auto base_type_id = ExprAsType(context, parse_node, base_type_expr_id);
- base_type_id = context.AsCompleteType(base_type_id, [&] {
- CARBON_DIAGNOSTIC(IncompleteTypeInBaseDecl, Error,
- "Base `{0}` is an incomplete type.", std::string);
- return context.emitter().Build(
- parse_node, IncompleteTypeInBaseDecl,
- context.sem_ir().StringifyType(base_type_id));
- });
-
- if (base_type_id != SemIR::TypeId::Error) {
- // For now, we treat all types that aren't introduced by a `class`
- // declaration as being final classes.
- // TODO: Once we have a better idea of which types are considered to be
- // classes, produce a better diagnostic for deriving from a non-class type.
- auto base_class = context.types().TryGetAs<SemIR::ClassType>(base_type_id);
- if (!base_class ||
- context.classes().Get(base_class->class_id).inheritance_kind ==
- SemIR::Class::Final) {
- CARBON_DIAGNOSTIC(BaseIsFinal, Error,
- "Deriving from final type `{0}`. Base type must be an "
- "`abstract` or `base` class.",
- std::string);
- context.emitter().Emit(parse_node, BaseIsFinal,
- context.sem_ir().StringifyType(base_type_id));
- }
- }
+ auto base_info = CheckBaseType(context, parse_node, base_type_expr_id);
 
  // The `base` value in the class scope has an unbound element type. Instance
  // binding will be performed when it's found by name lookup into an instance.
  auto field_type_inst_id = context.AddInst(SemIR::UnboundElementType{
  parse_node, context.GetBuiltinType(SemIR::BuiltinKind::TypeType),
- class_info.self_type_id, base_type_id});
+ class_info.self_type_id, base_info.type_id});
  auto field_type_id = context.CanonicalizeType(field_type_inst_id);
  class_info.base_id = context.AddInst(SemIR::BaseDecl{
- parse_node, field_type_id, base_type_id,
+ parse_node, field_type_id, base_info.type_id,
  SemIR::ElementIndex(
  context.args_type_info_stack().PeekCurrentBlockContents().size())});
 
  // Add a corresponding field to the object representation of the class.
  // TODO: Consider whether we want to use `partial T` here.
- context.args_type_info_stack().AddInst(
- SemIR::StructTypeField{parse_node, SemIR::NameId::Base, base_type_id});
+ context.args_type_info_stack().AddInst(SemIR::StructTypeField{
+ parse_node, SemIR::NameId::Base, base_info.type_id});
 
  // Bind the name `base` in the class to the base field.
  context.decl_name_stack().AddNameToLookup(
  context.decl_name_stack().MakeUnqualifiedName(parse_node,
  SemIR::NameId::Base),
  class_info.base_id);
+
+ // Extend the class scope with the base class.
+ if (!!(modifiers & KeywordModifierSet::Extend)) {
+ auto& class_scope = context.name_scopes().Get(class_info.scope_id);
+ if (base_info.scope_id.is_valid()) {
+ class_scope.extended_scopes.push_back(base_info.scope_id);
+ } else {
+ class_scope.has_error = true;
+ }
+ }
  return true;
 }
 

toolchain/check/handle_name.cpp

@@ -131,8 +131,9 @@ auto HandleMemberAccessExpr(Context& context, Parse::NodeId parse_node)
  if (auto unbound_element_type =
  context.types().TryGetAs<SemIR::UnboundElementType>(
  member_type_id)) {
- // TODO: Check that the unbound element type describes a member of this
- // class. Perform a conversion of the base if necessary.
+ // Convert the base to the type of the element if necessary.
+ base_id = ConvertToValueOrRefOfType(
+ context, parse_node, base_id, unbound_element_type->class_type_id);
 
  // Find the specified element, which could be either a field or a base
  // class, and build an element access expression.

toolchain/check/testdata/class/base.carbon

@@ -67,6 +67,7 @@ fn Access(d: Derived) -> (i32, i32) {
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT: .base = %.loc12_20.2
 // CHECK:STDOUT: .d = %d
+// CHECK:STDOUT: extend name_scope1
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @Make() -> %return: Derived {