From 85e9642d18f44371bff7502dbbf53d3d69f86ea6 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 23 Oct 2023 11:04:01 -0700 Subject: [PATCH] Lower types in the order they were completed. (#3324) This is a prerequisite for class support, where a class can be referenced as a type before it becomes complete. For example, given: ```carbon class A { fn F(a: A); class B {} var b: B; } fn A.F(a: A) {} ``` we need to lower `B` before we lower `A`, even though `A` is used as a type first. This will also start catching some cases where we don't require a type to be complete despite using it, as we now only lower types that are required to be complete. Remove the poison values for struct and tuple literals. We don't need those any more, because we never generate references to those literals as values, and we don't have a type to use for them because we never require the type of a literal to be complete, only the type of the entity initialized by the literal, which can be different, for example when initializing an array from a tuple literal or a class from a struct literal. This doesn't affect the output: `llvm::Type` objects that are not referenced by an LLVM module don't affect the IR for that module, and the order in which `llvm::Type`s are created doesn't affect anything either. --- toolchain/check/context.cpp | 9 +++++++++ toolchain/lower/file_context.cpp | 14 ++++++++------ toolchain/lower/file_context.h | 1 + toolchain/lower/function_context.h | 6 ++++-- toolchain/lower/handle.cpp | 13 +++++-------- toolchain/sem_ir/file.h | 11 +++++++++++ 6 files changed, 38 insertions(+), 16 deletions(-) diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 179c09d61e8b..50ed030a0618 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -370,6 +370,15 @@ class TypeCompleter { work_list_.push_back( {value_rep.type_id, Phase::BuildValueRepresentation}); } + // For a pointer representation, the pointee also needs to be complete. + if (value_rep.kind == SemIR::ValueRepresentation::Pointer) { + auto pointee_type_id = + context_.semantics_ir().GetPointeeType(value_rep.type_id); + if (!context_.semantics_ir().IsTypeComplete(pointee_type_id)) { + work_list_.push_back( + {pointee_type_id, Phase::BuildValueRepresentation}); + } + } break; } } diff --git a/toolchain/lower/file_context.cpp b/toolchain/lower/file_context.cpp index 590739d94621..cedd619a5127 100644 --- a/toolchain/lower/file_context.cpp +++ b/toolchain/lower/file_context.cpp @@ -31,11 +31,13 @@ FileContext::FileContext(llvm::LLVMContext& llvm_context, auto FileContext::Run() -> std::unique_ptr { CARBON_CHECK(llvm_module_) << "Run can only be called once."; - // Lower types. - auto types = semantics_ir_->types().array_ref(); - types_.resize_for_overwrite(types.size()); - for (auto [i, type] : llvm::enumerate(types)) { - types_[i] = BuildType(type.node_id); + // Lower all types that were required to be complete. Note that this may + // leave some entries in `types_` null, if those types were mentioned but not + // used. + types_.resize(semantics_ir_->types().size()); + for (auto type_id : semantics_ir_->complete_types()) { + types_[type_id.index] = + BuildType(semantics_ir_->types().Get(type_id).node_id); } // Lower function declarations. @@ -259,7 +261,7 @@ auto FileContext::BuildType(SemIR::NodeId node_id) -> llvm::Type* { return llvm::StructType::get(*llvm_context_, subtypes); } default: { - CARBON_FATAL() << "Cannot use node as type: " << node_id; + CARBON_FATAL() << "Cannot use node as type: " << node_id << " " << node; } } } diff --git a/toolchain/lower/file_context.h b/toolchain/lower/file_context.h index 9e19c96c1333..f60f2d123565 100644 --- a/toolchain/lower/file_context.h +++ b/toolchain/lower/file_context.h @@ -38,6 +38,7 @@ class FileContext { return GetTypeType(); } CARBON_CHECK(type_id.index >= 0) << type_id; + CARBON_CHECK(types_[type_id.index]) << "Missing type " << type_id; return types_[type_id.index]; } diff --git a/toolchain/lower/function_context.h b/toolchain/lower/function_context.h index ba0cc4535ddf..1daa2438d347 100644 --- a/toolchain/lower/function_context.h +++ b/toolchain/lower/function_context.h @@ -47,14 +47,16 @@ class FunctionContext { } auto it = locals_.find(node_id); - CARBON_CHECK(it != locals_.end()) << "Missing local: " << node_id; + CARBON_CHECK(it != locals_.end()) << "Missing local: " << node_id << " " + << semantics_ir().GetNode(node_id); return it->second; } // Sets the value for the given node. auto SetLocal(SemIR::NodeId node_id, llvm::Value* value) { bool added = locals_.insert({node_id, value}).second; - CARBON_CHECK(added) << "Duplicate local insert: " << node_id; + CARBON_CHECK(added) << "Duplicate local insert: " << node_id << " " + << semantics_ir().GetNode(node_id); } // Gets a callable's function. diff --git a/toolchain/lower/handle.cpp b/toolchain/lower/handle.cpp index 62c2553f6bea..6d75fe81e262 100644 --- a/toolchain/lower/handle.cpp +++ b/toolchain/lower/handle.cpp @@ -355,12 +355,11 @@ auto HandleStructAccess(FunctionContext& context, SemIR::NodeId node_id, node.type_id, member_name)); } -auto HandleStructLiteral(FunctionContext& context, SemIR::NodeId node_id, - SemIR::StructLiteral node) -> void { +auto HandleStructLiteral(FunctionContext& /*context*/, + SemIR::NodeId /*node_id*/, + SemIR::StructLiteral /*node*/) -> void { // A StructLiteral should always be converted to a StructInit or StructValue // if its value is needed. - context.SetLocal(node_id, - llvm::PoisonValue::get(context.GetType(node.type_id))); } // Emits the value representation for a struct or tuple whose elements are the @@ -470,12 +469,10 @@ auto HandleTupleIndex(FunctionContext& context, SemIR::NodeId node_id, node.type_id, "tuple.index")); } -auto HandleTupleLiteral(FunctionContext& context, SemIR::NodeId node_id, - SemIR::TupleLiteral node) -> void { +auto HandleTupleLiteral(FunctionContext& /*context*/, SemIR::NodeId /*node_id*/, + SemIR::TupleLiteral /*node*/) -> void { // A TupleLiteral should always be converted to a TupleInit or TupleValue if // its value is needed. - context.SetLocal(node_id, - llvm::PoisonValue::get(context.GetType(node.type_id))); } auto HandleTupleInit(FunctionContext& context, SemIR::NodeId node_id, diff --git a/toolchain/sem_ir/file.h b/toolchain/sem_ir/file.h index 41104fe2ea04..84a660a2d06e 100644 --- a/toolchain/sem_ir/file.h +++ b/toolchain/sem_ir/file.h @@ -262,6 +262,7 @@ class File : public Printable { ValueRepresentation::Unknown) << "Type " << object_type_id << " completed more than once"; types().Get(object_type_id).value_representation = value_representation; + complete_types_.push_back(object_type_id); } auto GetTypeAllowBuiltinTypes(TypeId type_id) const -> NodeId { @@ -356,6 +357,13 @@ class File : public Printable { auto nodes_size() const -> int { return nodes_.size(); } auto node_blocks_size() const -> int { return node_blocks_.size(); } + // A list of types that were completed in this file, in the order in which + // they were completed. Earlier types in this list cannot contain instances of + // later types. + auto complete_types() const -> llvm::ArrayRef { + return complete_types_; + } + auto top_node_block_id() const -> NodeBlockId { return top_node_block_id_; } auto set_top_node_block_id(NodeBlockId block_id) -> void { top_node_block_id_ = block_id; @@ -416,6 +424,9 @@ class File : public Printable { // Descriptions of types used in this file. ValueStore types_; + // Types that were completed in this file. + llvm::SmallVector complete_types_; + // Type blocks within the IR. These reference entries in types_. Storage for // the data is provided by allocator_. llvm::SmallVector> type_blocks_;