Merging CrossRef into ImportRefUsed, shifting builtins over. (#3659)

This is a bit of a cleanup; I probably should've just renamed CrossRef
instead of adding ImportRefUsed.

Adding `is_builtin` to InstId is more about providing a standard API for
the check, which I expect to add a little more of.

Shifts import tests to validate that the BuildValueRepr CHECK isn't
accidentally hit.
This commit is contained in:
Jon Ross-Perkins
2024-01-27 00:36:58 +00:00
committed by GitHub
parent 13de9e9d06
commit 8167c44a03
17 changed files with 116 additions and 95 deletions
+13 -15
View File
@@ -787,19 +787,17 @@ class TypeCompleter {
return value_rep;
};
auto BuildCrossRefValueRepr(SemIR::TypeId type_id, SemIR::CrossRef xref) const
auto BuildImportRefUsedValueRepr(SemIR::TypeId type_id,
SemIR::ImportRefUsed import_ref) const
-> SemIR::ValueRepr {
auto xref_inst =
context_.cross_ref_irs().Get(xref.ir_id)->insts().Get(xref.inst_id);
CARBON_CHECK(import_ref.inst_id.is_builtin())
<< "TODO: Handle non-builtin ImportRefUsed cases, such as functions, "
"classes, and interfaces";
// The canonical description of a type should only have cross-references
// for entities owned by another File, such as builtins, which are owned
// by the prelude, and named entities like classes and interfaces, which
// we don't support yet.
CARBON_CHECK(xref_inst.kind() == SemIR::Builtin::Kind)
<< "TODO: Handle other kinds of inst cross-references";
const auto& import_ir = context_.cross_ref_irs().Get(import_ref.ir_id);
auto import_inst = import_ir->insts().Get(import_ref.inst_id);
switch (xref_inst.As<SemIR::Builtin>().builtin_kind) {
switch (import_inst.As<SemIR::Builtin>().builtin_kind) {
case SemIR::BuiltinKind::TypeType:
case SemIR::BuiltinKind::Error:
case SemIR::BuiltinKind::Invalid:
@@ -945,7 +943,6 @@ class TypeCompleter {
case SemIR::InterfaceDecl::Kind:
case SemIR::IntLiteral::Kind:
case SemIR::ImportRefUnused::Kind:
case SemIR::ImportRefUsed::Kind:
case SemIR::NameRef::Kind:
case SemIR::Namespace::Kind:
case SemIR::Param::Kind:
@@ -972,9 +969,6 @@ class TypeCompleter {
case SemIR::VarStorage::Kind:
CARBON_FATAL() << "Type refers to non-type inst " << inst;
case SemIR::CrossRef::Kind:
return BuildCrossRefValueRepr(type_id, inst.As<SemIR::CrossRef>());
case SemIR::ArrayType::Kind: {
// For arrays, it's convenient to always use a pointer representation,
// even when the array has zero or one element, in order to support
@@ -982,6 +976,10 @@ class TypeCompleter {
return MakePointerValueRepr(type_id, SemIR::ValueRepr::ObjectAggregate);
}
case SemIR::ImportRefUsed::Kind:
return BuildImportRefUsedValueRepr(type_id,
inst.As<SemIR::ImportRefUsed>());
case SemIR::StructType::Kind:
return BuildStructTypeValueRepr(type_id, inst.As<SemIR::StructType>());
@@ -1000,7 +998,7 @@ class TypeCompleter {
SemIR::ValueRepr::ObjectAggregate);
case SemIR::Builtin::Kind:
CARBON_FATAL() << "Builtins should be named as cross-references";
CARBON_FATAL() << "Builtins should be named as ImportRefUsed";
case SemIR::BindSymbolicName::Kind:
case SemIR::PointerType::Kind:
+5 -3
View File
@@ -402,8 +402,8 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst)
// TODO: These need special handling.
case SemIR::BindValue::Kind:
case SemIR::Call::Kind:
case SemIR::CrossRef::Kind:
case SemIR::Deref::Kind:
case SemIR::ImportRefUsed::Kind:
case SemIR::Temporary::Kind:
case SemIR::TemporaryStorage::Kind:
case SemIR::ValueAsRef::Kind:
@@ -473,8 +473,6 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst)
case SemIR::BranchWithArg::Kind:
case SemIR::ClassDecl::Kind:
case SemIR::Import::Kind:
case SemIR::ImportRefUnused::Kind:
case SemIR::ImportRefUsed::Kind:
case SemIR::InterfaceDecl::Kind:
case SemIR::Param::Kind:
case SemIR::ReturnExpr::Kind:
@@ -483,6 +481,10 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst)
case SemIR::TupleLiteral::Kind:
case SemIR::VarStorage::Kind:
break;
case SemIR::ImportRefUnused::Kind:
CARBON_FATAL() << "ImportRefUnused should transform to ImportRefUsed "
"before TryEvalInst.";
}
return SemIR::ConstantId::NotConstant;
}
+9 -9
View File
@@ -19,15 +19,15 @@
// CHECK:STDOUT: type0: {constant: template instNamespaceType, value_rep: {kind: copy, type: type0}}
// CHECK:STDOUT: type_blocks: {}
// CHECK:STDOUT: insts:
// CHECK:STDOUT: instTypeType: {kind: CrossRef, arg0: ir0, arg1: instTypeType, type: typeTypeType}
// CHECK:STDOUT: instError: {kind: CrossRef, arg0: ir0, arg1: instError, type: typeError}
// CHECK:STDOUT: instBoolType: {kind: CrossRef, arg0: ir0, arg1: instBoolType, type: typeTypeType}
// CHECK:STDOUT: instIntType: {kind: CrossRef, arg0: ir0, arg1: instIntType, type: typeTypeType}
// CHECK:STDOUT: instFloatType: {kind: CrossRef, arg0: ir0, arg1: instFloatType, type: typeTypeType}
// CHECK:STDOUT: instStringType: {kind: CrossRef, arg0: ir0, arg1: instStringType, type: typeTypeType}
// CHECK:STDOUT: instFunctionType: {kind: CrossRef, arg0: ir0, arg1: instFunctionType, type: typeTypeType}
// CHECK:STDOUT: instBoundMethodType: {kind: CrossRef, arg0: ir0, arg1: instBoundMethodType, type: typeTypeType}
// CHECK:STDOUT: instNamespaceType: {kind: CrossRef, arg0: ir0, arg1: instNamespaceType, type: typeTypeType}
// CHECK:STDOUT: instTypeType: {kind: ImportRefUsed, arg0: ir0, arg1: instTypeType, type: typeTypeType}
// CHECK:STDOUT: instError: {kind: ImportRefUsed, arg0: ir0, arg1: instError, type: typeError}
// CHECK:STDOUT: instBoolType: {kind: ImportRefUsed, arg0: ir0, arg1: instBoolType, type: typeTypeType}
// CHECK:STDOUT: instIntType: {kind: ImportRefUsed, arg0: ir0, arg1: instIntType, type: typeTypeType}
// CHECK:STDOUT: instFloatType: {kind: ImportRefUsed, arg0: ir0, arg1: instFloatType, type: typeTypeType}
// CHECK:STDOUT: instStringType: {kind: ImportRefUsed, arg0: ir0, arg1: instStringType, type: typeTypeType}
// CHECK:STDOUT: instFunctionType: {kind: ImportRefUsed, arg0: ir0, arg1: instFunctionType, type: typeTypeType}
// CHECK:STDOUT: instBoundMethodType: {kind: ImportRefUsed, arg0: ir0, arg1: instBoundMethodType, type: typeTypeType}
// CHECK:STDOUT: instNamespaceType: {kind: ImportRefUsed, arg0: ir0, arg1: instNamespaceType, type: typeTypeType}
// CHECK:STDOUT: inst+0: {kind: Namespace, arg0: name_scope0, arg1: inst<invalid>, type: type0}
// CHECK:STDOUT: constant_values:
// CHECK:STDOUT: instTypeType: template instTypeType
@@ -3,6 +3,8 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
// AUTOUPDATE
// CHECK:STDERR: b.carbon: ERROR: Semantics TODO: `TODO: ResolveIfImportRefUnused for ClassDecl`.
// CHECK:STDERR: b.carbon: ERROR: Semantics TODO: `TODO: ResolveIfImportRefUnused for ClassDecl`.
// --- a.carbon
@@ -23,7 +25,11 @@ library "b" api;
import library "a";
// TODO: When ready, consider tests of basic import functionality.
fn Run() {
var x: Empty = {};
var y: ForwardDeclared = {};
y.F();
}
// CHECK:STDOUT: --- a.carbon
// CHECK:STDOUT:
@@ -56,9 +62,30 @@ import library "a";
// CHECK:STDOUT:
// CHECK:STDOUT: --- b.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %.1: type = struct_type {} [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace {.Empty = %import_ref.1, .ForwardDeclared = %import_ref.2} [template]
// CHECK:STDOUT: %import_ref.1 = import_ref ir1, inst+1, unused
// CHECK:STDOUT: %import_ref.2 = import_ref ir1, inst+4, unused
// CHECK:STDOUT: package: <namespace> = namespace {.Empty = %import_ref.1, .ForwardDeclared = %import_ref.2, .Run = %Run} [template]
// CHECK:STDOUT: %import_ref.1: <error> = import_ref ir1, inst+1, used
// CHECK:STDOUT: %import_ref.2: <error> = import_ref ir1, inst+4, used
// CHECK:STDOUT: %Run: <function> = fn_decl @Run [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Run() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %Empty.ref: <error> = name_ref Empty, file.%import_ref.1
// CHECK:STDOUT: %x.var: ref <error> = var x
// CHECK:STDOUT: %x: ref <error> = bind_name x, %x.var
// CHECK:STDOUT: %.loc7: {} = struct_literal ()
// CHECK:STDOUT: assign %x.var, <error>
// CHECK:STDOUT: %ForwardDeclared.ref: <error> = name_ref ForwardDeclared, file.%import_ref.2
// CHECK:STDOUT: %y.var: ref <error> = var y
// CHECK:STDOUT: %y: ref <error> = bind_name y, %y.var
// CHECK:STDOUT: %.loc8: {} = struct_literal ()
// CHECK:STDOUT: assign %y.var, <error>
// CHECK:STDOUT: %y.ref: ref <error> = name_ref y, %y
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
@@ -3,12 +3,13 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
// AUTOUPDATE
// CHECK:STDERR: b.carbon: ERROR: Semantics TODO: `TODO: ResolveIfImportRefUnused for FunctionDecl`.
// --- a.carbon
library "a" api;
fn F() {}
fn F();
// --- b.carbon
@@ -16,7 +17,9 @@ library "b" api;
import library "a";
// TODO: When ready, consider tests of basic import functionality.
fn Run() {
F();
}
// CHECK:STDOUT: --- a.carbon
// CHECK:STDOUT:
@@ -25,15 +28,19 @@ import library "a";
// CHECK:STDOUT: %F: <function> = fn_decl @F [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT: fn @F();
// CHECK:STDOUT:
// CHECK:STDOUT: --- b.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace {.F = %import_ref} [template]
// CHECK:STDOUT: %import_ref = import_ref ir1, inst+1, unused
// CHECK:STDOUT: package: <namespace> = namespace {.F = %import_ref, .Run = %Run} [template]
// CHECK:STDOUT: %import_ref: <error> = import_ref ir1, inst+1, used
// CHECK:STDOUT: %Run: <function> = fn_decl @Run [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Run() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %F.ref: <error> = name_ref F, file.%import_ref
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
@@ -3,12 +3,13 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
// AUTOUPDATE
// CHECK:STDERR: b.carbon: ERROR: Semantics TODO: `TODO: ResolveIfImportRefUnused for FunctionDecl`.
// --- a.carbon
library "a" api;
fn F();
fn F() {}
// --- b.carbon
@@ -16,7 +17,9 @@ library "b" api;
import library "a";
// TODO: When ready, consider tests of basic import functionality.
fn Run() {
F();
}
// CHECK:STDOUT: --- a.carbon
// CHECK:STDOUT:
@@ -25,12 +28,22 @@ import library "a";
// CHECK:STDOUT: %F: <function> = fn_decl @F [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F();
// CHECK:STDOUT: fn @F() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: --- b.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace {.F = %import_ref} [template]
// CHECK:STDOUT: %import_ref = import_ref ir1, inst+1, unused
// CHECK:STDOUT: package: <namespace> = namespace {.F = %import_ref, .Run = %Run} [template]
// CHECK:STDOUT: %import_ref: <error> = import_ref ir1, inst+1, used
// CHECK:STDOUT: %Run: <function> = fn_decl @Run [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Run() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %F.ref: <error> = name_ref F, file.%import_ref
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
+2 -2
View File
@@ -42,12 +42,12 @@ var a: () = a_ref;
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace {.a_ref = %import_ref, .a = %a} [template]
// CHECK:STDOUT: %import_ref: <error> = import_ref ir1, inst+2, used
// CHECK:STDOUT: %import_ref: ref <error> = import_ref ir1, inst+2, used
// CHECK:STDOUT: %.loc4_9.1: () = tuple_literal ()
// CHECK:STDOUT: %.loc4_9.2: type = converted %.loc4_9.1, constants.%.1 [template = constants.%.1]
// CHECK:STDOUT: %a.var: ref () = var a
// CHECK:STDOUT: %a: ref () = bind_name a, %a.var
// CHECK:STDOUT: %a_ref.ref: <error> = name_ref a_ref, %import_ref
// CHECK:STDOUT: %a_ref.ref: ref <error> = name_ref a_ref, %import_ref
// CHECK:STDOUT: assign %a.var, <error>
// CHECK:STDOUT: }
// CHECK:STDOUT:
+1 -1
View File
@@ -57,7 +57,7 @@ auto FileContext::Run() -> std::unique_ptr<llvm::Module> {
auto FileContext::GetGlobal(SemIR::InstId inst_id) -> llvm::Value* {
// All builtins are types, with the same empty lowered value.
if (inst_id.index < SemIR::BuiltinKind::ValidCount) {
if (inst_id.is_builtin()) {
return GetTypeAsValue();
}
+1 -1
View File
@@ -41,7 +41,7 @@ class FunctionContext {
// Returns a value for the given instruction.
auto GetValue(SemIR::InstId inst_id) -> llvm::Value* {
// All builtins are types, with the same empty lowered value.
if (inst_id.index < SemIR::BuiltinKind::ValidCount) {
if (inst_id.is_builtin()) {
return GetTypeAsValue();
}
-5
View File
@@ -180,11 +180,6 @@ auto HandleConverted(FunctionContext& context, SemIR::InstId inst_id,
context.SetLocal(inst_id, context.GetValue(inst.result_id));
}
auto HandleCrossRef(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/,
SemIR::CrossRef inst) -> void {
FatalErrorIfEncountered(inst);
}
auto HandleDeref(FunctionContext& context, SemIR::InstId inst_id,
SemIR::Deref inst) -> void {
context.SetLocal(inst_id, context.GetValue(inst.pointer_id));
+6 -15
View File
@@ -112,8 +112,7 @@ File::File(SharedValueStores& value_stores, std::string filename,
// special-cased values.
auto type_id = inst.type_id();
auto builtin_id = SemIR::InstId(i);
insts_.AddInNoBlock(
{Parse::NodeId::Invalid, CrossRef{type_id, BuiltinIR, builtin_id}});
insts_.AddInNoBlock({ImportRefUsed{type_id, BuiltinIR, builtin_id}});
constant_values_.Set(builtin_id,
SemIR::ConstantId::ForTemplateConstant(builtin_id));
}
@@ -215,12 +214,6 @@ static auto GetTypePrecedence(InstKind kind) -> int {
case PointerType::Kind:
return -2;
case CrossRef::Kind:
// TODO: Once we support stringification of cross-references, we'll need
// to determine the precedence of the target of the cross-reference. For
// now, all cross-references refer to builtin types from the prelude.
return 0;
case AddrOf::Kind:
case AddrPattern::Kind:
case ArrayIndex::Kind:
@@ -303,7 +296,7 @@ auto File::StringifyTypeExpr(InstId outer_inst_id) const -> std::string {
}
// Builtins have designated labels.
if (step.inst_id.index < BuiltinKind::ValidCount) {
if (step.inst_id.is_builtin()) {
out << BuiltinKind::FromInt(step.inst_id.index).label();
continue;
}
@@ -446,7 +439,6 @@ auto File::StringifyTypeExpr(InstId outer_inst_id) const -> std::string {
case ClassElementAccess::Kind:
case ClassInit::Kind:
case Converted::Kind:
case CrossRef::Kind:
case Deref::Kind:
case FieldDecl::Kind:
case FunctionDecl::Kind:
@@ -508,7 +500,6 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory {
case FunctionDecl::Kind:
case Import::Kind:
case ImportRefUnused::Kind:
case ImportRefUsed::Kind:
case InterfaceDecl::Kind:
case Namespace::Kind:
case Return::Kind:
@@ -516,10 +507,10 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory {
case StructTypeField::Kind:
return ExprCategory::NotExpr;
case CrossRef::Kind: {
auto xref = inst.As<CrossRef>();
ir = ir->cross_ref_irs().Get(xref.ir_id);
inst_id = xref.inst_id;
case ImportRefUsed::Kind: {
auto import_ref = inst.As<ImportRefUsed>();
ir = ir->cross_ref_irs().Get(import_ref.ir_id);
inst_id = import_ref.inst_id;
continue;
}
+2 -2
View File
@@ -350,8 +350,8 @@ class File : public Printable<File> {
// the data is provided by allocator_.
BlockValueStore<TypeBlockId> type_blocks_;
// All instructions. The first entries will always be cross-references to
// builtins, at indices matching BuiltinKind ordering.
// All instructions. The first entries will always be ImportRefs to builtins,
// at indices matching BuiltinKind ordering.
InstStore insts_;
// Constant values for instructions.
+1 -7
View File
@@ -169,7 +169,7 @@ class InstNamer {
}
// Check for a builtin.
if (inst_id.index < BuiltinKind::ValidCount) {
if (inst_id.is_builtin()) {
return BuiltinKind::FromInt(inst_id.index).label().str();
}
@@ -940,12 +940,6 @@ class Formatter {
FormatReturnSlot(init.dest_id);
}
auto FormatInstructionRHS(CrossRef inst) -> void {
// TODO: Figure out a way to make this meaningful. We'll need some way to
// name cross-reference IRs, perhaps by the instruction ID of the import?
out_ << " " << inst.ir_id << ", " << inst.inst_id;
}
auto FormatInstructionRHS(ImportRefUnused inst) -> void {
// Don't format the inst_id because it refers to a different IR.
// TODO: Consider a better way to format the InstID from other IRs.
+10 -3
View File
@@ -37,18 +37,25 @@ struct InstId : public IdBase, public Printable<InstId> {
// The namespace for a `package` expression.
static const InstId PackageNamespace;
// Returns the cross-reference instruction ID for a builtin. This relies on
// File guarantees for builtin cross-reference placement.
// Returns the instruction ID for a builtin. This relies on File guarantees
// for builtin ImportRefUsed placement.
static constexpr auto ForBuiltin(BuiltinKind kind) -> InstId {
return InstId(kind.AsInt());
}
using IdBase::IdBase;
// Returns true if the instruction is a builtin. Requires is_valid.
auto is_builtin() const -> bool {
CARBON_CHECK(is_valid());
return index < BuiltinKind::ValidCount;
}
auto Print(llvm::raw_ostream& out) const -> void {
out << "inst";
if (!is_valid()) {
IdBase::Print(out);
} else if (index < BuiltinKind::ValidCount) {
} else if (is_builtin()) {
out << BuiltinKind::FromInt(index);
} else {
// Use the `+` as a small reminder that this is a delta, rather than an
-1
View File
@@ -41,7 +41,6 @@ CARBON_SEM_IR_INST_KIND(ClassInit)
CARBON_SEM_IR_INST_KIND(ClassType)
CARBON_SEM_IR_INST_KIND(ConstType)
CARBON_SEM_IR_INST_KIND(Converted)
CARBON_SEM_IR_INST_KIND(CrossRef)
CARBON_SEM_IR_INST_KIND(Deref)
CARBON_SEM_IR_INST_KIND(FieldDecl)
CARBON_SEM_IR_INST_KIND(FunctionDecl)
-12
View File
@@ -364,18 +364,6 @@ struct Converted {
InstId result_id;
};
// A cross-reference between IRs.
struct CrossRef {
static constexpr auto Kind = InstKind::CrossRef.Define<Parse::NodeId>("xref");
// No parse node: an instruction's parse tree node must refer to a node in the
// current parse tree. This cannot use the cross-referenced instruction's
// parse tree node because it will be in a different parse tree.
TypeId type_id;
CrossRefIRId ir_id;
InstId inst_id;
};
struct Deref {
// TODO: Make Parse::NodeId more specific.
static constexpr auto Kind = InstKind::Deref.Define<Parse::NodeId>("deref");
+2 -2
View File
@@ -38,8 +38,8 @@ TEST(SemIRTest, YAML) {
d.RunCommand(
{"compile", "--phase=check", "--dump-raw-sem-ir", "test.carbon"});
// Matches the ID of an instruction. The numbers may change because of builtin
// cross-references, so this code is only doing loose structural checks.
// Matches the ID of an instruction. Instruction counts may change as various
// support changes, so this code is only doing loose structural checks.
auto int_id = Yaml::Scalar(MatchesRegex(R"(int\d+)"));
auto inst_id = Yaml::Scalar(MatchesRegex(R"(inst\+\d+)"));
auto constant_id =