diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 786bd8cafdec..5c2e2f2ed253 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -73,7 +73,8 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info) -> void { // Define the package scope, with an instruction for `package` expressions to // reference. - auto package_scope_id = context.name_scopes().Add(); + auto package_scope_id = + context.name_scopes().Add(SemIR::InstId::PackageNamespace); CARBON_CHECK(package_scope_id == SemIR::NameScopeId::Package); auto package_inst = context.AddInst(SemIR::Namespace{ diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index 669c446217c4..61113601ccca 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/toolchain/check/decl_name_stack.cpp @@ -69,11 +69,22 @@ auto DeclNameStack::LookupOrAddName(NameContext name_context, context_->AddNameToLookup(name_context.parse_node, name_context.unresolved_name_id, target_id); } else { - // TODO: Reject unless the scope is a namespace scope or the name is - // unqualified. - bool success = context_->name_scopes().AddEntry( - name_context.target_scope_id, name_context.unresolved_name_id, - target_id); + auto& name_scope = + context_->name_scopes().Get(name_context.target_scope_id); + if (name_context.has_qualifiers) { + auto inst = context_->insts().Get(name_scope.inst_id); + if (!inst.Is()) { + // TODO: Point at the declaration for the scoped entity. + CARBON_DIAGNOSTIC( + QualifiedDeclOutsideScopeEntity, Error, + "Out-of-line declaration requires a declaration in " + "scoped entity."); + context_->emitter().Emit(name_context.parse_node, + QualifiedDeclOutsideScopeEntity); + } + } + auto [_, success] = name_scope.names.insert( + {name_context.unresolved_name_id, target_id}); CARBON_CHECK(success) << "Duplicate names should have been resolved previously: " << name_context.unresolved_name_id << " in " @@ -102,7 +113,7 @@ auto DeclNameStack::ApplyNameQualifier(Parse::NodeId parse_node, auto DeclNameStack::ApplyNameQualifierTo(NameContext& name_context, Parse::NodeId parse_node, SemIR::NameId name_id) -> void { - if (CanResolveQualifier(name_context, parse_node)) { + if (TryResolveQualifier(name_context, parse_node)) { // For identifier nodes, we need to perform a lookup on the identifier. auto resolved_inst_id = context_->LookupNameInDecl( name_context.parse_node, name_id, name_context.target_scope_id); @@ -152,8 +163,12 @@ auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context) -> void { } } -auto DeclNameStack::CanResolveQualifier(NameContext& name_context, +auto DeclNameStack::TryResolveQualifier(NameContext& name_context, Parse::NodeId parse_node) -> bool { + // Update has_qualifiers based on the state before any possible changes. If + // this is the first qualifier, it may just be the name. + name_context.has_qualifiers = name_context.state != NameContext::State::Empty; + switch (name_context.state) { case NameContext::State::Error: // Already in an error state, so return without examining. diff --git a/toolchain/check/decl_name_stack.h b/toolchain/check/decl_name_stack.h index d98b200e3776..a56a4f039f58 100644 --- a/toolchain/check/decl_name_stack.h +++ b/toolchain/check/decl_name_stack.h @@ -110,6 +110,9 @@ class DeclNameStack { State state = State::Empty; + // Whether there have been qualifiers in the name. + bool has_qualifiers = false; + // The scope which qualified names are added to. For unqualified names in // an unnamed scope, this will be Invalid to indicate the current scope // should be used. @@ -182,7 +185,7 @@ class DeclNameStack { // Returns true if the context is in a state where it can resolve qualifiers. // Updates name_context as needed. - auto CanResolveQualifier(NameContext& name_context, Parse::NodeId parse_node) + auto TryResolveQualifier(NameContext& name_context, Parse::NodeId parse_node) -> bool; // Updates the scope on name_context as needed. This is called after diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index eedec4710560..948ea3190827 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -136,7 +136,7 @@ auto HandleClassDefinitionStart(Context& context, Parse::NodeId parse_node) .Emit(); } else { class_info.definition_id = class_decl_id; - class_info.scope_id = context.name_scopes().Add(); + class_info.scope_id = context.name_scopes().Add(class_decl_id); } // Enter the class scope. diff --git a/toolchain/check/handle_interface.cpp b/toolchain/check/handle_interface.cpp index 9dd1b7256847..e0feeb794045 100644 --- a/toolchain/check/handle_interface.cpp +++ b/toolchain/check/handle_interface.cpp @@ -109,7 +109,7 @@ auto HandleInterfaceDefinitionStart(Context& context, Parse::NodeId parse_node) .Emit(); } else { interface_info.definition_id = interface_decl_id; - interface_info.scope_id = context.name_scopes().Add(); + interface_info.scope_id = context.name_scopes().Add(interface_decl_id); } // Enter the interface scope. diff --git a/toolchain/check/handle_namespace.cpp b/toolchain/check/handle_namespace.cpp index 4445dd27e41e..ebeced48b506 100644 --- a/toolchain/check/handle_namespace.cpp +++ b/toolchain/check/handle_namespace.cpp @@ -5,6 +5,7 @@ #include "toolchain/check/context.h" #include "toolchain/check/decl_state.h" #include "toolchain/check/modifiers.h" +#include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/inst.h" namespace Carbon::Check { @@ -21,9 +22,12 @@ auto HandleNamespace(Context& context, Parse::NodeId parse_node) -> bool { auto name_context = context.decl_name_stack().FinishName(); LimitModifiersOnDecl(context, KeywordModifierSet::None, Lex::TokenKind::Namespace); - auto namespace_id = context.AddInst(SemIR::Namespace{ + auto namespace_inst = SemIR::Namespace{ parse_node, context.GetBuiltinType(SemIR::BuiltinKind::NamespaceType), - context.name_scopes().Add()}); + SemIR::NameScopeId::Invalid}; + auto namespace_id = context.AddInst(namespace_inst); + namespace_inst.name_scope_id = context.name_scopes().Add(namespace_id); + context.insts().Set(namespace_id, namespace_inst); context.decl_name_stack().AddNameToLookup(name_context, namespace_id); context.decl_name_stack().PopScope(); diff --git a/toolchain/check/testdata/class/fail_base_method_define.carbon b/toolchain/check/testdata/class/fail_base_method_define.carbon index db6fb70a196d..d186fe7aa990 100644 --- a/toolchain/check/testdata/class/fail_base_method_define.carbon +++ b/toolchain/check/testdata/class/fail_base_method_define.carbon @@ -16,7 +16,9 @@ class D { extend base: B; } -// TODO: This should be rejected. +// CHECK:STDERR: fail_base_method_define.carbon:[[@LINE+3]]:6: ERROR: Out-of-line declaration requires a declaration in scoped entity. +// CHECK:STDERR: fn D.F() {} +// CHECK:STDERR: ^ fn D.F() {} // CHECK:STDERR: fail_base_method_define.carbon:[[@LINE+3]]:6: ERROR: Name `C` not found. @@ -40,7 +42,7 @@ fn D.C.F() {} // CHECK:STDOUT: %D.decl = class_decl @D, () // CHECK:STDOUT: %D: type = class_type @D // CHECK:STDOUT: %F: = fn_decl @F.3 -// CHECK:STDOUT: %.loc25: = fn_decl @.1 +// CHECK:STDOUT: %.loc27: = fn_decl @.1 // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: class @B { diff --git a/toolchain/check/testdata/class/fail_out_of_line_decl.carbon b/toolchain/check/testdata/class/fail_out_of_line_decl.carbon new file mode 100644 index 000000000000..627409e0eca1 --- /dev/null +++ b/toolchain/check/testdata/class/fail_out_of_line_decl.carbon @@ -0,0 +1,37 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE + +class C {} + +// CHECK:STDERR: fail_out_of_line_decl.carbon:[[@LINE+3]]:6: ERROR: Out-of-line declaration requires a declaration in scoped entity. +// CHECK:STDERR: fn C.F() {} +// CHECK:STDERR: ^ +fn C.F() {} + +// CHECK:STDOUT: --- fail_out_of_line_decl.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %.loc7: type = struct_type {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.C = %C.decl} +// CHECK:STDOUT: %C.decl = class_decl @C, () +// CHECK:STDOUT: %C: type = class_type @C +// CHECK:STDOUT: %F: = fn_decl @F +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @C { +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .F = file.%F +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 52cce33e09f8..0828219bf201 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -214,6 +214,7 @@ CARBON_DIAGNOSTIC_KIND(ReturnStatementMissingExpr) CARBON_DIAGNOSTIC_KIND(ImplicitAsConversionFailure) CARBON_DIAGNOSTIC_KIND(ExplicitAsConversionFailure) CARBON_DIAGNOSTIC_KIND(TypeExprEvaluationFailure) +CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsideScopeEntity) CARBON_DIAGNOSTIC_KIND(QualifiedDeclInIncompleteClassScope) CARBON_DIAGNOSTIC_KIND(QualifiedDeclInNonScope) CARBON_DIAGNOSTIC_KIND(QualifiedDeclNonScopeEntity) diff --git a/toolchain/sem_ir/value_stores.h b/toolchain/sem_ir/value_stores.h index aaef8b951e4b..d1d353305ff5 100644 --- a/toolchain/sem_ir/value_stores.h +++ b/toolchain/sem_ir/value_stores.h @@ -173,7 +173,7 @@ class NameStoreWrapper { struct NameScope { // Names in the scope. - llvm::DenseMap names; + llvm::DenseMap names = llvm::DenseMap(); // Scopes extended by this scope. // @@ -193,6 +193,9 @@ struct NameScope { // TODO: Consider using something like `TinyPtrVector` for this. llvm::SmallVector extended_scopes; + // The instructioning which owns the scope. + InstId inst_id; + // Whether we have diagnosed an error in a construct that would have added // names to this scope. For example, this can happen if an `import` failed or // an `extend` declaration was ill-formed. If true, the `names` map is @@ -205,7 +208,9 @@ struct NameScope { class NameScopeStore { public: // Adds a name scope, returning an ID to reference it. - auto Add() -> NameScopeId { return values_.AddDefaultValue(); } + auto Add(InstId inst_id) -> NameScopeId { + return values_.Add({.inst_id = inst_id}); + } // Adds an entry to a name scope. Returns true on success, false on // duplicates.