diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index a7fd484f29b5..2767e681a80c 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -57,12 +57,11 @@ struct UnitInfo { llvm::SmallVector imports; }; - explicit UnitInfo(SemIR::CheckIRId check_ir_id, Unit& unit, - Parse::NodeLocConverter& converter) + explicit UnitInfo(SemIR::CheckIRId check_ir_id, Unit& unit) : check_ir_id(check_ir_id), unit(&unit), err_tracker(*unit.consumer), - emitter(converter, err_tracker) {} + emitter(*unit.node_converter, err_tracker) {} SemIR::CheckIRId check_ir_id; Unit* unit; @@ -91,6 +90,9 @@ struct UnitInfo { // The corresponding `api` unit if this is an `impl` file. The entry should // also be in the corresponding `PackageImports`. UnitInfo* api_for_impl = nullptr; + + // Whether the unit has been checked. + bool is_checked = false; }; } // namespace @@ -101,7 +103,7 @@ static auto CollectDirectImports(llvm::SmallVector& results, const UnitInfo::PackageImports& imports, bool is_local) -> void { for (const auto& import : imports.imports) { - const auto& direct_ir = **import.unit_info->unit->sem_ir; + const auto& direct_ir = *import.unit_info->unit->sem_ir; auto& index = ir_to_result_index[direct_ir.check_ir_id().index]; if (index != -1) { // This should only happen when doing API imports for an implementation @@ -339,10 +341,9 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info, auto import_decl_id = context.AddInst( names.node_id, {.package_id = SemIR::NameId::ForIdentifier(names.package_id)}); - SetApiImportIR(context, - {.decl_id = import_decl_id, - .is_export = false, - .sem_ir = &**unit_info.api_for_impl->unit->sem_ir}); + SetApiImportIR(context, {.decl_id = import_decl_id, + .is_export = false, + .sem_ir = unit_info.api_for_impl->unit->sem_ir}); } else { SetApiImportIR(context, {.decl_id = SemIR::InstId::Invalid, .sem_ir = nullptr}); @@ -360,7 +361,7 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info, // Process the imports. if (unit_info.api_for_impl) { ImportApiFile(context, namespace_type_id, - **unit_info.api_for_impl->unit->sem_ir); + *unit_info.api_for_impl->unit->sem_ir); } ImportCurrentPackage(context, unit_info, total_ir_count, package_inst_id, namespace_type_id); @@ -477,27 +478,16 @@ static auto ProcessNodeIds(Context& context, llvm::raw_ostream* vlog_stream, } // Produces and checks the IR for the provided Parse::Tree. -static auto CheckParseTree( - llvm::MutableArrayRef node_converters, - UnitInfo& unit_info, int total_ir_count, llvm::raw_ostream* vlog_stream) - -> void { +static auto CheckParseTree(UnitInfo& unit_info, int total_ir_count, + llvm::raw_ostream* vlog_stream) -> void { Timings::ScopedTiming timing(unit_info.unit->timings, "check"); - auto package_id = IdentifierId::Invalid; - auto library_id = StringLiteralValueId::Invalid; - if (const auto& packaging = unit_info.unit->parse_tree->packaging_decl()) { - package_id = packaging->names.package_id; - library_id = packaging->names.library_id; - } - unit_info.unit->sem_ir->emplace( - unit_info.check_ir_id, package_id, - SemIR::LibraryNameId::ForStringLiteralValueId(library_id), - *unit_info.unit->value_stores, - unit_info.unit->tokens->source().filename().str()); + // We can safely mark this as checked at the start. + unit_info.is_checked = true; - SemIR::File& sem_ir = **unit_info.unit->sem_ir; - SemIRDiagnosticConverter converter(node_converters, &sem_ir); - Context::DiagnosticEmitter emitter(converter, unit_info.err_tracker); + SemIR::File& sem_ir = *unit_info.unit->sem_ir; + Context::DiagnosticEmitter emitter(*unit_info.unit->sem_ir_converter, + unit_info.err_tracker); Context context(*unit_info.unit->tokens, emitter, *unit_info.unit->parse_tree, unit_info.unit->get_parse_tree_and_subtrees, sem_ir, vlog_stream); @@ -514,7 +504,7 @@ static auto CheckParseTree( ImportImplsFromApiFile(context); if (!ProcessNodeIds(context, vlog_stream, unit_info.err_tracker, - node_converters[unit_info.check_ir_id.index])) { + *unit_info.unit->node_converter)) { context.sem_ir().set_has_errors(true); return; } @@ -796,16 +786,14 @@ static auto BuildApiMapAndDiagnosePackaging( return api_map; } -auto CheckParseTrees( - llvm::MutableArrayRef units, - llvm::MutableArrayRef node_converters, - bool prelude_import, llvm::raw_ostream* vlog_stream) -> void { +auto CheckParseTrees(llvm::MutableArrayRef units, bool prelude_import, + llvm::raw_ostream* vlog_stream) -> void { // UnitInfo is big due to its SmallVectors, so we default to 0 on the // stack. llvm::SmallVector unit_infos; unit_infos.reserve(units.size()); for (auto [i, unit] : llvm::enumerate(units)) { - unit_infos.emplace_back(SemIR::CheckIRId(i), unit, node_converters[i]); + unit_infos.emplace_back(SemIR::CheckIRId(i), unit); } Map api_map = @@ -854,7 +842,7 @@ auto CheckParseTrees( for (int check_index = 0; check_index < static_cast(ready_to_check.size()); ++check_index) { auto* unit_info = ready_to_check[check_index]; - CheckParseTree(node_converters, *unit_info, units.size(), vlog_stream); + CheckParseTree(*unit_info, units.size(), vlog_stream); for (auto* incoming_import : unit_info->incoming_imports) { --incoming_import->imports_remaining; if (incoming_import->imports_remaining == 0) { @@ -875,7 +863,7 @@ auto CheckParseTrees( for (auto& package_imports : unit_info.package_imports) { for (auto* import_it = package_imports.imports.begin(); import_it != package_imports.imports.end();) { - if (*import_it->unit_info->unit->sem_ir) { + if (import_it->unit_info->is_checked) { // The import is checked, so continue. ++import_it; } else { @@ -901,7 +889,7 @@ auto CheckParseTrees( // incomplete imports. for (auto& unit_info : unit_infos) { if (unit_info.imports_remaining > 0) { - CheckParseTree(node_converters, unit_info, units.size(), vlog_stream); + CheckParseTree(unit_info, units.size(), vlog_stream); } } } diff --git a/toolchain/check/check.h b/toolchain/check/check.h index fbb6cdfe39de..502aec146c8b 100644 --- a/toolchain/check/check.h +++ b/toolchain/check/check.h @@ -19,23 +19,28 @@ namespace Carbon::Check { // Checking information that's tracked per file. struct Unit { + DiagnosticConsumer* consumer; SharedValueStores* value_stores; std::optional* timings; const Lex::TokenizedBuffer* tokens; const Parse::Tree* parse_tree; - DiagnosticConsumer* consumer; + // Returns a lazily constructed TreeAndSubtrees. - std::function get_parse_tree_and_subtrees; - // The generated IR. Unset on input, set on output. - std::optional* sem_ir; + llvm::function_ref + get_parse_tree_and_subtrees; + + // The unit's SemIR, provided as empty and filled in by CheckParseTrees. + SemIR::File* sem_ir; + + // Diagnostic converters. + Parse::NodeLocConverter* node_converter; + SemIRDiagnosticConverter* sem_ir_converter; }; // Checks a group of parse trees. This will use imports to decide the order of // checking. -auto CheckParseTrees( - llvm::MutableArrayRef units, - llvm::MutableArrayRef node_converters, - bool prelude_import, llvm::raw_ostream* vlog_stream) -> void; +auto CheckParseTrees(llvm::MutableArrayRef units, bool prelude_import, + llvm::raw_ostream* vlog_stream) -> void; } // namespace Carbon::Check diff --git a/toolchain/check/sem_ir_diagnostic_converter.cpp b/toolchain/check/sem_ir_diagnostic_converter.cpp index 7c18b7cb8589..b4acabdb948f 100644 --- a/toolchain/check/sem_ir_diagnostic_converter.cpp +++ b/toolchain/check/sem_ir_diagnostic_converter.cpp @@ -173,7 +173,7 @@ auto SemIRDiagnosticConverter::ConvertLocInFile(const SemIR::File* sem_ir, bool token_only, ContextFnT context_fn) const -> DiagnosticLoc { - return node_converters_[sem_ir->check_ir_id().index].ConvertLoc( + return node_converters_[sem_ir->check_ir_id().index]->ConvertLoc( Parse::NodeLoc(node_id, token_only), context_fn); } diff --git a/toolchain/check/sem_ir_diagnostic_converter.h b/toolchain/check/sem_ir_diagnostic_converter.h index bae5f2a44822..34743ebd0781 100644 --- a/toolchain/check/sem_ir_diagnostic_converter.h +++ b/toolchain/check/sem_ir_diagnostic_converter.h @@ -17,7 +17,7 @@ namespace Carbon::Check { class SemIRDiagnosticConverter : public DiagnosticConverter { public: explicit SemIRDiagnosticConverter( - llvm::ArrayRef node_converters, + llvm::ArrayRef node_converters, const SemIR::File* sem_ir) : node_converters_(node_converters), sem_ir_(sem_ir) {} @@ -38,7 +38,7 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { -> DiagnosticLoc; // Converters for each SemIR. - llvm::ArrayRef node_converters_; + llvm::ArrayRef node_converters_; // The current SemIR being processed. const SemIR::File* sem_ir_; diff --git a/toolchain/driver/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp index 66a2ada561d7..45603c779d0f 100644 --- a/toolchain/driver/compile_subcommand.cpp +++ b/toolchain/driver/compile_subcommand.cpp @@ -395,7 +395,7 @@ class CompilationUnit { // Parses tokens. Returns true on success. auto RunParse() -> void { - CARBON_CHECK(tokens_); + CARBON_CHECK(tokens_, "Must call RunLex first"); LogCall("Parse::Parse", "parse", [&] { parse_tree_ = Parse::Parse(*tokens_, *consumer_, vlog_stream_); @@ -418,34 +418,49 @@ class CompilationUnit { } } + auto PreCheck() -> Parse::NodeLocConverter& { + CARBON_CHECK(parse_tree_, "Must call RunParse first"); + get_parse_tree_and_subtrees_ = [this]() -> const Parse::TreeAndSubtrees& { + return this->GetParseTreeAndSubtrees(); + }; + node_converter_.emplace(&*tokens_, source_->filename(), + *get_parse_tree_and_subtrees_); + return *node_converter_; + } + // Returns information needed to check this unit. - auto GetCheckUnit() -> Check::Unit { - CARBON_CHECK(parse_tree_); - return { - .value_stores = &value_stores_, - .timings = &timings_, - .tokens = &*tokens_, - .parse_tree = &*parse_tree_, - .consumer = consumer_, - .get_parse_tree_and_subtrees = [&]() -> const Parse::TreeAndSubtrees& { - return GetParseTreeAndSubtrees(); - }, - .sem_ir = &sem_ir_}; + auto GetCheckUnit(SemIR::CheckIRId check_ir_id, + llvm::ArrayRef node_converters) + -> Check::Unit { + CARBON_CHECK(node_converter_, "Must call PreCheck first"); + + sem_ir_.emplace(check_ir_id, parse_tree_->packaging_decl(), value_stores_, + input_filename_); + if (mem_usage_) { + mem_usage_->Collect("sem_ir_", *sem_ir_); + } + + sem_ir_converter_.emplace(node_converters, &*sem_ir_); + return {.consumer = consumer_, + .value_stores = &value_stores_, + .timings = &timings_, + .tokens = &*tokens_, + .parse_tree = &*parse_tree_, + .get_parse_tree_and_subtrees = *get_parse_tree_and_subtrees_, + .sem_ir = &*sem_ir_, + .node_converter = &*node_converter_, + .sem_ir_converter = &*sem_ir_converter_}; } // Runs post-check logic. Returns true if checking succeeded for the IR. auto PostCheck() -> void { - CARBON_CHECK(sem_ir_); + CARBON_CHECK(sem_ir_converter_, "Must call GetCheckUnit first"); // We've finished all steps that can produce diagnostics. Emit the // diagnostics now, so that the developer sees them sooner and doesn't need // to wait for code generation. consumer_->Flush(); - if (mem_usage_) { - mem_usage_->Collect("sem_ir_", *sem_ir_); - } - if (options_.dump_raw_sem_ir && IncludeInDumps()) { CARBON_VLOG("*** Raw SemIR::File ***\n{0}\n", *sem_ir_); sem_ir_->Print(driver_env_->output_stream, options_.builtin_sem_ir); @@ -486,8 +501,8 @@ class CompilationUnit { } // Lower SemIR to LLVM IR. - auto RunLower(const Check::SemIRDiagnosticConverter& converter) -> void { - CARBON_CHECK(sem_ir_); + auto RunLower() -> void { + CARBON_CHECK(sem_ir_converter_, "Must call PostCheck first"); LogCall("Lower::LowerToLLVM", "lower", [&] { llvm_context_ = std::make_unique(); @@ -495,8 +510,8 @@ class CompilationUnit { // producing textual LLVM IR. SemIR::InstNamer inst_namer(*tokens_, *parse_tree_, *sem_ir_); module_ = Lower::LowerToLLVM(*llvm_context_, options_.include_debug_info, - converter, input_filename_, *sem_ir_, - &inst_namer, vlog_stream_); + *sem_ir_converter_, input_filename_, + *sem_ir_, &inst_namer, vlog_stream_); }); if (vlog_stream_) { CARBON_VLOG("*** llvm::Module ***\n"); @@ -511,7 +526,7 @@ class CompilationUnit { } auto RunCodeGen() -> void { - CARBON_CHECK(module_); + CARBON_CHECK(module_, "Must call RunLower first"); LogCall("CodeGen", "codegen", [&] { success_ = RunCodeGenHelper(); }); } @@ -577,8 +592,8 @@ class CompilationUnit { if (!source_->is_regular_file()) { // Don't invent file names like `-.o` or `/dev/stdin.o`. driver_env_->error_stream - << "error: output file name must be specified for input '" - << input_filename_ << "' that is not a regular file\n"; + << "error: output file name must be specified for input `" + << input_filename_ << "` that is not a regular file\n"; return false; } output_filename = input_filename_; @@ -654,6 +669,10 @@ class CompilationUnit { DriverEnv* driver_env_; SharedValueStores value_stores_; const CompileOptions& options_; + // The input filename from the command line. For most diagnostics, we + // typically use `source_->filename()`, which includes a `-` -> `` + // translation. However, logging and some diagnostics use the command line + // argument. std::string input_filename_; // Copied from driver_ for CARBON_VLOG. @@ -675,6 +694,10 @@ class CompilationUnit { std::optional tokens_; std::optional parse_tree_; std::optional parse_tree_and_subtrees_; + std::optional> + get_parse_tree_and_subtrees_; + std::optional node_converter_; + std::optional sem_ir_converter_; std::optional sem_ir_; std::unique_ptr llvm_context_; std::unique_ptr module_; @@ -779,22 +802,30 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { return make_result(); } - // Check. - SharedValueStores builtin_value_stores; - llvm::SmallVector check_units; + // Pre-check assigns IR IDs and constructs node converters. + llvm::SmallVector node_converters; + // This size may not match due to units that are missing source, but that's an + // error case and not worth extra work. + node_converters.reserve(units.size()); for (auto& unit : units) { if (unit->has_source()) { - check_units.push_back(unit->GetCheckUnit()); + node_converters.push_back(&unit->PreCheck()); } } - llvm::SmallVector node_converters; - node_converters.reserve(check_units.size()); - for (auto& unit : check_units) { - node_converters.emplace_back(unit.tokens, unit.tokens->source().filename(), - unit.get_parse_tree_and_subtrees); + + // Gather Check::Units. + llvm::SmallVector check_units; + check_units.reserve(node_converters.size()); + for (auto& unit : units) { + if (unit->has_source()) { + SemIR::CheckIRId check_ir_id(check_units.size()); + check_units.push_back(unit->GetCheckUnit(check_ir_id, node_converters)); + } } + + // Execute the actual checking. CARBON_VLOG_TO(driver_env.vlog_stream, "*** Check::CheckParseTrees ***\n"); - Check::CheckParseTrees(check_units, node_converters, options_.prelude_import, + Check::CheckParseTrees(check_units, options_.prelude_import, driver_env.vlog_stream); CARBON_VLOG_TO(driver_env.vlog_stream, "*** Check::CheckParseTrees done ***\n"); @@ -816,9 +847,7 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { // Lower. for (const auto& unit : units) { - Check::SemIRDiagnosticConverter converter(node_converters, - &**unit->GetCheckUnit().sem_ir); - unit->RunLower(converter); + unit->RunLower(); } if (options_.phase == CompileOptions::Phase::Lower) { return make_result(); diff --git a/toolchain/driver/testdata/fail_missing_stdin_output.carbon b/toolchain/driver/testdata/fail_missing_stdin_output.carbon index bda372a084d9..a29d3c0e362b 100644 --- a/toolchain/driver/testdata/fail_missing_stdin_output.carbon +++ b/toolchain/driver/testdata/fail_missing_stdin_output.carbon @@ -9,4 +9,4 @@ // TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_missing_stdin_output.carbon // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_missing_stdin_output.carbon -// CHECK:STDERR: error: output file name must be specified for input '-' that is not a regular file +// CHECK:STDERR: error: output file name must be specified for input `-` that is not a regular file diff --git a/toolchain/driver/testdata/stdin.carbon b/toolchain/driver/testdata/stdin.carbon index 247cc27ce8a9..d9173db3456e 100644 --- a/toolchain/driver/testdata/stdin.carbon +++ b/toolchain/driver/testdata/stdin.carbon @@ -10,7 +10,7 @@ // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/stdin.carbon -// CHECK:STDOUT: --- +// CHECK:STDOUT: --- - // CHECK:STDOUT: // CHECK:STDOUT: file { // CHECK:STDOUT: package: = namespace [template] {} diff --git a/toolchain/sem_ir/BUILD b/toolchain/sem_ir/BUILD index 3422f088997c..cec202cc8279 100644 --- a/toolchain/sem_ir/BUILD +++ b/toolchain/sem_ir/BUILD @@ -118,6 +118,7 @@ cc_library( "//toolchain/base:yaml", "//toolchain/lex:token_kind", "//toolchain/parse:node_kind", + "//toolchain/parse:tree", "@llvm-project//llvm:Support", ], ) diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp index ef90fd3ca3fc..772ccb7a6a52 100644 --- a/toolchain/sem_ir/file.cpp +++ b/toolchain/sem_ir/file.cpp @@ -19,12 +19,15 @@ namespace Carbon::SemIR { -File::File(CheckIRId check_ir_id, IdentifierId package_id, - LibraryNameId library_id, SharedValueStores& value_stores, - std::string filename) +File::File(CheckIRId check_ir_id, + const std::optional& packaging_decl, + SharedValueStores& value_stores, std::string filename) : check_ir_id_(check_ir_id), - package_id_(package_id), - library_id_(library_id), + package_id_(packaging_decl ? packaging_decl->names.package_id + : IdentifierId::Invalid), + library_id_(packaging_decl ? LibraryNameId::ForStringLiteralValueId( + packaging_decl->names.library_id) + : LibraryNameId::Default), value_stores_(&value_stores), filename_(std::move(filename)), impls_(*this), diff --git a/toolchain/sem_ir/file.h b/toolchain/sem_ir/file.h index 3324dd32be28..3770fb46cfc8 100644 --- a/toolchain/sem_ir/file.h +++ b/toolchain/sem_ir/file.h @@ -14,6 +14,7 @@ #include "toolchain/base/shared_value_stores.h" #include "toolchain/base/value_store.h" #include "toolchain/base/yaml.h" +#include "toolchain/parse/tree.h" #include "toolchain/sem_ir/class.h" #include "toolchain/sem_ir/constant.h" #include "toolchain/sem_ir/entity_name.h" @@ -43,9 +44,9 @@ class File : public Printable { }; // Starts a new file for Check::CheckParseTree. - explicit File(CheckIRId check_ir_id, IdentifierId package_id, - LibraryNameId library_id, SharedValueStores& value_stores, - std::string filename); + explicit File(CheckIRId check_ir_id, + const std::optional& packaging_decl, + SharedValueStores& value_stores, std::string filename); File(const File&) = delete; auto operator=(const File&) -> File& = delete;