Skip to content

Commit

Permalink
Fix crash lowering an imported impl method.
Browse files Browse the repository at this point in the history
While we don't need a lookup table for an imported impl from a different
library, we do still need to import the name scope so we can compute the
parent scope for mangling purposes.
  • Loading branch information
zygoloid committed Nov 5, 2024
1 parent f03bd6a commit 1911569
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 14 deletions.
32 changes: 18 additions & 14 deletions toolchain/check/import_ref.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1803,23 +1803,27 @@ class ImportRefResolver {

new_impl.witness_id = witness_id;

// Import the definition scope, if we might need it. Name lookup is never
// performed into this scope by a user of the impl, so this is only
// necessary in the same library that defined the impl, in order to support
// defining members of the impl out of line in the impl file when the impl
// is defined in the API file.
if (import_ir_id_ == SemIR::ImportIRId::ApiForImpl) {
if (import_impl.scope_id.is_valid()) {
new_impl.scope_id = context_.name_scopes().Add(
new_impl.first_owning_decl_id, SemIR::NameId::Invalid,
new_impl.parent_scope_id);
auto& new_scope = context_.name_scopes().Get(new_impl.scope_id);
const auto& import_scope =
import_ir_.name_scopes().Get(import_impl.scope_id);

// Push a block so that we can add scoped instructions to it.
context_.inst_block_stack().Push();
AddNameScopeImportRefs(import_scope, new_scope);
new_impl.body_block_id = context_.inst_block_stack().Pop();
// Import the contents of the definition scope, if we might need it. Name
// lookup is never performed into this scope by a user of the impl, so
// this is only necessary in the same library that defined the impl, in
// order to support defining members of the impl out of line in the impl
// file when the impl is defined in the API file.
// TODO: Check to see if this impl is owned by the API file, rather than
// merely being imported into it.
if (import_ir_id_ == SemIR::ImportIRId::ApiForImpl) {
auto& new_scope = context_.name_scopes().Get(new_impl.scope_id);
const auto& import_scope =
import_ir_.name_scopes().Get(import_impl.scope_id);

// Push a block so that we can add scoped instructions to it.
context_.inst_block_stack().Push();
AddNameScopeImportRefs(import_scope, new_scope);
new_impl.body_block_id = context_.inst_block_stack().Pop();
}
}
}

Expand Down
81 changes: 81 additions & 0 deletions toolchain/lower/testdata/impl/import.carbon
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// 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
// TIP: To test this file alone, run:
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lower/testdata/impl/import.carbon
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lower/testdata/impl/import.carbon

// --- impl_def.carbon

library "[[@TEST_NAME]]";

interface I {
fn F[self: Self]() -> i32;
}

class A {
var n: i32;
}

impl A as I {
fn F[self: A]() -> i32 {
return self.n;
}
}

// --- impl_use.carbon

import library "impl_def";

fn Call(a: A) -> i32 {
return a.(I.F)();
}

// CHECK:STDOUT: ; ModuleID = 'impl_def.carbon'
// CHECK:STDOUT: source_filename = "impl_def.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: define i32 @"_CF.A.Main:I.Main"(ptr %self) !dbg !4 {
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %.loc14_16.1.n = getelementptr inbounds nuw { i32 }, ptr %self, i32 0, i32 0, !dbg !7
// CHECK:STDOUT: %.loc14_16.2 = load i32, ptr %.loc14_16.1.n, align 4, !dbg !7
// CHECK:STDOUT: ret i32 %.loc14_16.2, !dbg !8
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: !llvm.module.flags = !{!0, !1}
// CHECK:STDOUT: !llvm.dbg.cu = !{!2}
// CHECK:STDOUT:
// CHECK:STDOUT: !0 = !{i32 7, !"Dwarf Version", i32 5}
// CHECK:STDOUT: !1 = !{i32 2, !"Debug Info Version", i32 3}
// CHECK:STDOUT: !2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
// CHECK:STDOUT: !3 = !DIFile(filename: "impl_def.carbon", directory: "")
// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "F", linkageName: "_CF.A.Main:I.Main", scope: null, file: !3, line: 13, type: !5, spFlags: DISPFlagDefinition, unit: !2)
// CHECK:STDOUT: !5 = !DISubroutineType(types: !6)
// CHECK:STDOUT: !6 = !{}
// CHECK:STDOUT: !7 = !DILocation(line: 14, column: 12, scope: !4)
// CHECK:STDOUT: !8 = !DILocation(line: 14, column: 5, scope: !4)
// CHECK:STDOUT: ; ModuleID = 'impl_use.carbon'
// CHECK:STDOUT: source_filename = "impl_use.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: define i32 @_CCall.Main(ptr %a) !dbg !4 {
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %F.call = call i32 @"_CF.A.Main:I.Main"(ptr %a), !dbg !7
// CHECK:STDOUT: ret i32 %F.call, !dbg !8
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: declare i32 @"_CF.A.Main:I.Main"(ptr)
// CHECK:STDOUT:
// CHECK:STDOUT: !llvm.module.flags = !{!0, !1}
// CHECK:STDOUT: !llvm.dbg.cu = !{!2}
// CHECK:STDOUT:
// CHECK:STDOUT: !0 = !{i32 7, !"Dwarf Version", i32 5}
// CHECK:STDOUT: !1 = !{i32 2, !"Debug Info Version", i32 3}
// CHECK:STDOUT: !2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
// CHECK:STDOUT: !3 = !DIFile(filename: "impl_use.carbon", directory: "")
// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "Call", linkageName: "_CCall.Main", scope: null, file: !3, line: 12, type: !5, spFlags: DISPFlagDefinition, unit: !2)
// CHECK:STDOUT: !5 = !DISubroutineType(types: !6)
// CHECK:STDOUT: !6 = !{}
// CHECK:STDOUT: !7 = !DILocation(line: 16, column: 10, scope: !4)
// CHECK:STDOUT: !8 = !DILocation(line: 16, column: 3, scope: !4)

0 comments on commit 1911569

Please sign in to comment.