diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index 4ed3ac891b4d..4dfa9ce3871f 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -3101,12 +3101,17 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, return ResolveResult::Retry(); } + auto witnesses_block_id = SemIR::InstBlockId::None; + if (inst.witnesses_block_id.has_value()) { + witnesses_block_id = + AddCanonicalWitnessesBlock(resolver.local_ir(), witnesses); + } + return ResolveResult::Deduplicated( resolver, {.type_id = resolver.local_types().GetTypeIdForTypeConstantId(type_id), .type_inst_id = type_inst_id, - .witnesses_block_id = GetLocalCanonicalInstBlockId( - resolver, inst.witnesses_block_id, witnesses)}); + .witnesses_block_id = witnesses_block_id}); } static auto TryResolveTypedInst(ImportRefResolver& resolver, diff --git a/toolchain/check/testdata/impl/import_canonical_witnesses.carbon b/toolchain/check/testdata/impl/import_canonical_witnesses.carbon new file mode 100644 index 000000000000..1e92e4a949d0 --- /dev/null +++ b/toolchain/check/testdata/impl/import_canonical_witnesses.carbon @@ -0,0 +1,82 @@ +// 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 +// +// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/full.carbon +// +// This test is producing a case where interface ordering will differ between +// interfaces.carbon and use.carbon, testing that witnesses are maintained in a +// canonical order. This tests a regression where the imported ordering would +// be incorrectly used, resulting in witnesses getting swapped and an opaque +// error for the `SimpleOp` lookup because `Empty`'s table is empty. +// +// The IR is generally not significant, but the important difference is as +// follows (if these become the same, it means the test is no longer effective): +// +// SET-CHECK-SUBSET +// CHECK:STDOUT: --- interfaces.carbon +// CHECK:STDOUT: %facet_type: type = facet_type <@SimpleOp & @Empty> [concrete] +// CHECK:STDOUT: --- use.carbon +// CHECK:STDOUT: %facet_type: type = facet_type <@Empty & @SimpleOp> [concrete] +// +// This check does not strictly exclude extra facet types, although that should +// create different instruction names. We also anticipate diagnostics should be +// produced if there's a significant error. +// +// NOAUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/import_canonical_witnesses.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/import_canonical_witnesses.carbon + + +// --- interfaces.carbon +//@include-in-dumps +library "[[@TEST_NAME]]"; + +// An interface with a trivial function and blanket impl for pointers. +interface SimpleOp { + fn Op[self: Self](); +} + +impl forall [T:! type] T as SimpleOp { + fn Op[self: Self]() {} +} + +// An empty interface with blanket impl for pointers. +interface Empty {} +impl forall [T:! type] T as Empty {} + +// Class with a use of `SimpleOp.Op`. +class InterfaceWrap(T:! SimpleOp & Empty) { + fn UseSimpleOp[self: Self](value:! T) { + value.(SimpleOp.Op)(); + } +} + +// Interface combining the above for an indirect `SimpleOp` use through `C`, +// particularly use an associated constant. +interface CombinedFacet { + let T:! SimpleOp & Empty; + fn GetInterfaceWrap[self: Self]() -> InterfaceWrap(T); +} + +// A wrapper providing `CombinedFacet`. +class TypeWrap(T:! type) { + impl as CombinedFacet where .T = T { + fn GetInterfaceWrap[self: Self]() -> InterfaceWrap(T) { + return {}; + } + } +} + +// --- use.carbon +//@include-in-dumps +library "[[@TEST_NAME]]"; +import library "interfaces"; + +class C {} + +fn F(t: TypeWrap(C), c:! C) { + t.(CombinedFacet.GetInterfaceWrap)().UseSimpleOp(c); +} diff --git a/toolchain/lower/testdata/impl/import_facet.carbon b/toolchain/lower/testdata/impl/import_facet.carbon new file mode 100644 index 000000000000..3fb4b408bf91 --- /dev/null +++ b/toolchain/lower/testdata/impl/import_facet.carbon @@ -0,0 +1,150 @@ +// 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 +// +// This is testing a facet type that combines two interfaces across multiple +// import boundaries. +// +// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/full.carbon +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lower/testdata/impl/import_facet.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lower/testdata/impl/import_facet.carbon + +// --- a.carbon +library "[[@TEST_NAME]]"; + +class C(T:! Core.Copy & Core.Destroy) { + fn GetC[self: Self]() -> T { + var value: T; + return value; + } +} + +// --- b.carbon +library "[[@TEST_NAME]]"; +export import library "a"; + +interface I { + let T:! Core.Copy & Core.Destroy; + fn GetI[self: Self]() -> C(T); +} + +// --- c.carbon +library "[[@TEST_NAME]]"; +export import library "b"; + +class D(T:! type) { + impl as I where .T = T* { + fn GetI[self: Self]() -> C(T*) { + return {}; + } + } +} + +// --- d.carbon +library "[[@TEST_NAME]]"; +import library "c"; + +fn F(d: D(i32)) -> i32* { + return d.(I.GetI)().GetC(); +} + +// CHECK:STDOUT: ; ModuleID = 'a.carbon' +// CHECK:STDOUT: source_filename = "a.carbon" +// 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_plus_plus, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug) +// CHECK:STDOUT: !3 = !DIFile(filename: "a.carbon", directory: "") +// CHECK:STDOUT: ; ModuleID = 'b.carbon' +// CHECK:STDOUT: source_filename = "b.carbon" +// 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_plus_plus, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug) +// CHECK:STDOUT: !3 = !DIFile(filename: "b.carbon", directory: "") +// CHECK:STDOUT: ; ModuleID = 'c.carbon' +// CHECK:STDOUT: source_filename = "c.carbon" +// 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_plus_plus, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug) +// CHECK:STDOUT: !3 = !DIFile(filename: "c.carbon", directory: "") +// CHECK:STDOUT: ; ModuleID = 'd.carbon' +// CHECK:STDOUT: source_filename = "d.carbon" +// CHECK:STDOUT: +// CHECK:STDOUT: @C.val.136.anon = internal constant {} zeroinitializer +// CHECK:STDOUT: +// CHECK:STDOUT: ; Function Attrs: nounwind +// CHECK:STDOUT: define ptr @_CF.Main(ptr %d) #0 !dbg !4 { +// CHECK:STDOUT: entry: +// CHECK:STDOUT: %.loc5_21.1.temp = alloca {}, align 8, !dbg !10 +// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %.loc5_21.1.temp), !dbg !10 +// CHECK:STDOUT: call void @"_CGetI.D.Main:I.Main.b88d1103f417c6d4"(ptr %.loc5_21.1.temp, ptr %d), !dbg !10 +// CHECK:STDOUT: %C.GetC.call = call ptr @_CGetC.C.Main.6d3edfd8901fffe5(ptr %.loc5_21.1.temp), !dbg !10 +// CHECK:STDOUT: ret ptr %C.GetC.call, !dbg !11 +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: ; Function Attrs: nocallback nofree nosync nounwind willreturn memory(argmem: readwrite) +// CHECK:STDOUT: declare void @llvm.lifetime.start.p0(ptr captures(none)) #1 +// CHECK:STDOUT: +// CHECK:STDOUT: ; Function Attrs: nounwind +// CHECK:STDOUT: define linkonce_odr void @"_CGetI.D.Main:I.Main.b88d1103f417c6d4"(ptr sret({}) %return, ptr %self) #0 !dbg !12 { +// CHECK:STDOUT: call void @llvm.memcpy.p0.p0.i64(ptr align 1 %return, ptr align 1 @C.val.136.anon, i64 0, i1 false), !dbg !16 +// CHECK:STDOUT: ret void, !dbg !16 +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: ; Function Attrs: nounwind +// CHECK:STDOUT: define linkonce_odr ptr @_CGetC.C.Main.6d3edfd8901fffe5(ptr %self) #0 !dbg !17 { +// CHECK:STDOUT: %1 = alloca ptr, align 8, !dbg !21 +// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %1), !dbg !21 +// CHECK:STDOUT: %2 = load ptr, ptr %1, align 8, !dbg !22 +// CHECK:STDOUT: ret ptr %2, !dbg !23 +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: ; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: readwrite) +// CHECK:STDOUT: declare void @llvm.memcpy.p0.p0.i64(ptr noalias writeonly captures(none), ptr noalias readonly captures(none), i64, i1 immarg) #2 +// CHECK:STDOUT: +// CHECK:STDOUT: attributes #0 = { nounwind } +// CHECK:STDOUT: attributes #1 = { nocallback nofree nosync nounwind willreturn memory(argmem: readwrite) } +// CHECK:STDOUT: attributes #2 = { nocallback nofree nounwind willreturn memory(argmem: readwrite) } +// 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_plus_plus, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug) +// CHECK:STDOUT: !3 = !DIFile(filename: "d.carbon", directory: "") +// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "F", linkageName: "_CF.Main", scope: null, file: !3, line: 4, type: !5, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !8) +// CHECK:STDOUT: !5 = !DISubroutineType(types: !6) +// CHECK:STDOUT: !6 = !{!7, !7} +// CHECK:STDOUT: !7 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 8) +// CHECK:STDOUT: !8 = !{!9} +// CHECK:STDOUT: !9 = !DILocalVariable(arg: 1, scope: !4, type: !7) +// CHECK:STDOUT: !10 = !DILocation(line: 5, column: 10, scope: !4) +// CHECK:STDOUT: !11 = !DILocation(line: 5, column: 3, scope: !4) +// CHECK:STDOUT: !12 = distinct !DISubprogram(name: "GetI", linkageName: "_CGetI.D.Main:I.Main.b88d1103f417c6d4", scope: null, file: !13, line: 6, type: !5, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !14) +// CHECK:STDOUT: !13 = !DIFile(filename: "c.carbon", directory: "") +// CHECK:STDOUT: !14 = !{!15} +// CHECK:STDOUT: !15 = !DILocalVariable(arg: 1, scope: !12, type: !7) +// CHECK:STDOUT: !16 = !DILocation(line: 7, column: 7, scope: !12) +// CHECK:STDOUT: !17 = distinct !DISubprogram(name: "GetC", linkageName: "_CGetC.C.Main.6d3edfd8901fffe5", scope: null, file: !18, line: 4, type: !5, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !19) +// CHECK:STDOUT: !18 = !DIFile(filename: "a.carbon", directory: "") +// CHECK:STDOUT: !19 = !{!20} +// CHECK:STDOUT: !20 = !DILocalVariable(arg: 1, scope: !17, type: !7) +// CHECK:STDOUT: !21 = !DILocation(line: 5, column: 5, scope: !17) +// CHECK:STDOUT: !22 = !DILocation(line: 6, column: 12, scope: !17) +// CHECK:STDOUT: !23 = !DILocation(line: 6, column: 5, scope: !17) diff --git a/toolchain/sem_ir/facet_type_info.cpp b/toolchain/sem_ir/facet_type_info.cpp index 590d15d1599d..fe216e8bd692 100644 --- a/toolchain/sem_ir/facet_type_info.cpp +++ b/toolchain/sem_ir/facet_type_info.cpp @@ -6,7 +6,10 @@ #include +#include "toolchain/base/kind_switch.h" +#include "toolchain/sem_ir/file.h" #include "toolchain/sem_ir/ids.h" +#include "toolchain/sem_ir/typed_insts.h" namespace Carbon::SemIR { @@ -232,4 +235,50 @@ IdentifiedFacetType::IdentifiedFacetType( SortAndDeduplicate(required_interfaces_, RequiredLess); } +auto AddCanonicalWitnessesBlock(File& sem_ir, + llvm::SmallVector& witnesses) + -> InstBlockId { + // Small blocks don't need to be sorted. + if (witnesses.size() <= 1) { + return sem_ir.inst_blocks().AddCanonical(witnesses); + } + + llvm::SmallVector> sortable; + sortable.reserve(witnesses.size()); + + // Produce the sorted order based on the witness's SpecificInterface. + for (auto witness_id : witnesses) { + auto inst = sem_ir.insts().Get(witness_id); + CARBON_KIND_SWITCH(inst) { + case CARBON_KIND(ImplWitness witness): { + auto table = + sem_ir.insts().GetAs(witness.witness_table_id); + sortable.push_back( + {sem_ir.impls().Get(table.impl_id).interface, witness_id}); + break; + } + case CARBON_KIND(LookupImplWitness witness): { + sortable.push_back({sem_ir.specific_interfaces().Get( + witness.query_specific_interface_id), + witness_id}); + break; + } + default: + CARBON_FATAL("Unhandled inst: {0}", inst); + } + } + llvm::sort(sortable, [](auto& lhs, auto& rhs) { + return ImplsLess(lhs.first, rhs.first); + }); + + // Update the original list with the new order (reusing to avoid an + // allocation). + for (auto [witness_id, sortable_entry] : + llvm::zip_equal(witnesses, sortable)) { + witness_id = sortable_entry.second; + } + + return sem_ir.inst_blocks().AddCanonical(witnesses); +} + } // namespace Carbon::SemIR diff --git a/toolchain/sem_ir/facet_type_info.h b/toolchain/sem_ir/facet_type_info.h index 8ccb92c6e3c4..3833a16973e6 100644 --- a/toolchain/sem_ir/facet_type_info.h +++ b/toolchain/sem_ir/facet_type_info.h @@ -15,6 +15,8 @@ namespace Carbon::SemIR { +class File; + #define CARBON_BUILTIN_CONSTRAINT_MASK(X) \ /* Verifies types can use the builtin `type.destroy`. */ \ X(TypeCanDestroy) @@ -211,8 +213,12 @@ inline auto CarbonHashValue(const FacetTypeInfo& value, uint64_t seed) return static_cast(hasher); } -// Declared: -// (Carbon::HashCode) (value_ = 12557349131240970624) +// Given an array of witnesses, sorts them to match the FacetTypeInfo ordering +// and returns the resulting block ID. This assumes witnesses have already been +// deduplicated, because it's mainly for imports. +auto AddCanonicalWitnessesBlock(File& sem_ir, + llvm::SmallVector& witnesses) + -> InstBlockId; } // namespace Carbon::SemIR