Handle out-of-line declarations. (#3536)

By adding an InstId to the NameScope, we can determine whether the
declaration is being added to a scoped entity (versus a namespace).

The choice of InstId on NameScope is chosen versus other solutions
because, for imports, we want to just have a list of InstIds to import
and, from those, get the containing namespaces for addition. Similar may
also be desirable for printing fully qualified names given a singular
InstId. That means an InstId must have a path to find enclosing name
scopes.

What we're looking at here is:

- NameScopeId knows its InstId. (done here)
- Inst knows the enclosing NameScopeId. (future work)
- To walk up enclosing scopes for an Inst:
  1. Fetch the Inst.
2. Find its enclosing NameScopeId (which will be per-declaration due to
Function etc complexity).
  3. Fetch the NameScope if not Package scope. (if Package scope, done)
  4. Use the InstId on the NameScope to go back to step 1.

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{

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<SemIR::Namespace>()) {
+            // 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.

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

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.

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.

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();

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: <function> = fn_decl @F.3
-// CHECK:STDOUT:   %.loc25: <function> = fn_decl @.1
+// CHECK:STDOUT:   %.loc27: <function> = fn_decl @.1
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @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> = namespace {.C = %C.decl}
+// CHECK:STDOUT:   %C.decl = class_decl @C, ()
+// CHECK:STDOUT:   %C: type = class_type @C
+// CHECK:STDOUT:   %F: <function> = 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:

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)

toolchain/sem_ir/value_stores.h

@@ -173,7 +173,7 @@ class NameStoreWrapper {
 
 struct NameScope {
   // Names in the scope.
-  llvm::DenseMap<NameId, InstId> names;
+  llvm::DenseMap<NameId, InstId> names = llvm::DenseMap<NameId, InstId>();
 
   // Scopes extended by this scope.
   //
@@ -193,6 +193,9 @@ struct NameScope {
   // TODO: Consider using something like `TinyPtrVector` for this.
   llvm::SmallVector<NameScopeId, 1> 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.