Switch compile functions to use options structs (#5742)

I've been mulling this mainly for the parameter complexity of
check/lower, but doing lex/parse for symmetry.

I'm motivated by the plan to move dumping for all of them into the
respective functions, because of discussion about llvm-verifier. That
basically would add another bool parameter (or more) to each of these.
My instinct is we're going to probably accrue a little more over time,
so I'm suggesting this as maybe adding the boundary a little simpler
and/or easier to read.

Note it may make sense to refactor a little further, e.g. maybe
Lower::Context could receive the full set of options and pick out what
it wants, but I figured creating the struct itself would be a decent
start.

I'm trying to put things into options when we can produce a reasonable
default if the user doesn't assign a value. I'm using an explicit
constructor so that values can be added without affecting every caller.

A different factoring would be to pass in everything through the param
struct, but that just felt weird when I was trying it out.

Removing `inst_namer` and `module_name` from `LowerToLLVM` params --
both of these can be inferred from `sem_ir`, and I'm not seeing a
particular reason to maintain them at the call site.

toolchain/check/check.cpp

@@ -325,9 +325,8 @@ static auto BuildApiMapAndDiagnosePackaging(
 auto CheckParseTrees(
     llvm::MutableArrayRef<Unit> units,
     llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
-    bool prelude_import, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
-    llvm::StringRef target, llvm::raw_ostream* vlog_stream, bool fuzzing)
-    -> void {
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs, llvm::StringRef target,
+    const CheckParseTreesOptions& options) -> void {
   // UnitAndImports is big due to its SmallVectors, so we default to 0 on the
   // stack.
   llvm::SmallVector<UnitAndImports, 0> unit_infos(
@@ -348,14 +347,14 @@ auto CheckParseTrees(
       // An `impl` has an implicit import of its `api`.
       auto implicit_names = packaging->names;
       implicit_names.package_id = PackageNameId::None;
-      TrackImport(api_map, nullptr, unit_info, implicit_names, fuzzing);
+      TrackImport(api_map, nullptr, unit_info, implicit_names, options.fuzzing);
     }
 
     Map<ImportKey, Parse::NodeId> explicit_import_map;
 
     // Add the prelude import. It's added to explicit_import_map so that it can
     // conflict with an explicit import of the prelude.
-    if (prelude_import &&
+    if (options.prelude_import &&
         !(packaging && packaging->names.package_id == PackageNameId::Core)) {
       auto prelude_id =
           unit_info.unit->value_stores->string_literal_values().Add("prelude");
@@ -363,11 +362,12 @@ auto CheckParseTrees(
                   {.node_id = Parse::NoneNodeId(),
                    .package_id = PackageNameId::Core,
                    .library_id = prelude_id},
-                  fuzzing);
+                  options.fuzzing);
     }
 
     for (const auto& import : unit_info.parse_tree().imports()) {
-      TrackImport(api_map, &explicit_import_map, unit_info, import, fuzzing);
+      TrackImport(api_map, &explicit_import_map, unit_info, import,
+                  options.fuzzing);
     }
 
     // If there were no imports, mark the file as ready to check for below.
@@ -381,7 +381,8 @@ auto CheckParseTrees(
   for (int check_index = 0;
        check_index < static_cast<int>(ready_to_check.size()); ++check_index) {
     auto* unit_info = ready_to_check[check_index];
-    CheckUnit(unit_info, tree_and_subtrees_getters, fs, target, vlog_stream)
+    CheckUnit(unit_info, tree_and_subtrees_getters, fs, target,
+              options.vlog_stream)
         .Run();
     for (auto* incoming_import : unit_info->incoming_imports) {
       --incoming_import->imports_remaining;
@@ -430,7 +431,7 @@ auto CheckParseTrees(
     for (auto& unit_info : unit_infos) {
       if (unit_info.imports_remaining > 0) {
         CheckUnit(&unit_info, tree_and_subtrees_getters, fs, target,
-                  vlog_stream)
+                  options.vlog_stream)
             .Run();
       }
     }

toolchain/check/check.h

@@ -31,14 +31,28 @@ struct Unit {
   std::unique_ptr<clang::ASTUnit>* cpp_ast;
 };
 
+struct CheckParseTreesOptions {
+  // Options must be set individually, not through initialization.
+  explicit CheckParseTreesOptions() = default;
+
+  // Whether to import the prelude.
+  bool prelude_import = false;
+
+  // If set, enables verbose output.
+  llvm::raw_ostream* vlog_stream = nullptr;
+
+  // Whether fuzzing is being run. Used to disable features we don't want to
+  // fuzz.
+  bool fuzzing = false;
+};
+
 // Checks a group of parse trees. This will use imports to decide the order of
 // checking.
 auto CheckParseTrees(
     llvm::MutableArrayRef<Unit> units,
     llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
-    bool prelude_import, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
-    llvm::StringRef target, llvm::raw_ostream* vlog_stream, bool fuzzing)
-    -> void;
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs, llvm::StringRef target,
+    const CheckParseTreesOptions& options) -> void;
 
 }  // namespace Carbon::Check
 

toolchain/driver/compile_subcommand.cpp

@@ -606,8 +606,11 @@ auto CompilationUnit::RunLex() -> void {
 
   CARBON_VLOG("*** SourceBuffer ***\n```\n{0}\n```\n", source_->text());
 
-  LogCall("Lex::Lex", "lex",
-          [&] { tokens_ = Lex::Lex(value_stores_, *source_, *consumer_); });
+  LogCall("Lex::Lex", "lex", [&] {
+    Lex::LexOptions options;
+    options.consumer = consumer_;
+    tokens_ = Lex::Lex(value_stores_, *source_, options);
+  });
   if (options_->dump_tokens && IncludeInDumps()) {
     consumer_->Flush();
     tokens_->Print(*driver_env_->output_stream,
@@ -624,7 +627,10 @@ auto CompilationUnit::RunLex() -> void {
 
 auto CompilationUnit::RunParse() -> void {
   LogCall("Parse::Parse", "parse", [&] {
-    parse_tree_ = Parse::Parse(*tokens_, *consumer_, vlog_stream_);
+    Parse::ParseOptions options;
+    options.consumer = consumer_;
+    options.vlog_stream = vlog_stream_;
+    parse_tree_ = Parse::Parse(*tokens_, options);
   });
   if (options_->dump_parse_tree && IncludeInDumps()) {
     consumer_->Flush();
@@ -717,16 +723,14 @@ auto CompilationUnit::PostCheck() -> void {
 auto CompilationUnit::RunLower() -> void {
   LogCall("Lower::LowerToLLVM", "lower", [&] {
     llvm_context_ = std::make_unique<llvm::LLVMContext>();
-    // TODO: Consider disabling instruction naming by default if we're not
-    // producing textual LLVM IR.
-    SemIR::InstNamer inst_namer(&*sem_ir_);
-    llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> subtrees =
-        cache_->tree_and_subtrees_getters();
-    module_ = Lower::LowerToLLVM(
-        *llvm_context_, driver_env_->fs,
-        options_->run_llvm_verifier ? driver_env_->error_stream : nullptr,
-        options_->include_debug_info, subtrees, input_filename_, *sem_ir_,
-        &inst_namer, vlog_stream_);
+    Lower::LowerToLLVMOptions options;
+    options.llvm_verifier_stream =
+        options_->run_llvm_verifier ? driver_env_->error_stream : nullptr;
+    options.want_debug_info = options_->include_debug_info;
+    options.vlog_stream = vlog_stream_;
+    module_ = Lower::LowerToLLVM(*llvm_context_, driver_env_->fs,
+                                 cache_->tree_and_subtrees_getters(), *sem_ir_,
+                                 options);
   });
   if (vlog_stream_) {
     CARBON_VLOG("*** llvm::Module ***\n");
@@ -977,10 +981,13 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult {
 
   // Execute the actual checking.
   CARBON_VLOG_TO(driver_env.vlog_stream, "*** Check::CheckParseTrees ***\n");
+  Check::CheckParseTreesOptions options;
+  options.prelude_import = options_.prelude_import;
+  options.vlog_stream = driver_env.vlog_stream;
+  options.fuzzing = driver_env.fuzzing;
   Check::CheckParseTrees(check_units, cache.tree_and_subtrees_getters(),
-                         options_.prelude_import, driver_env.fs,
-                         options_.codegen_options.target,
-                         driver_env.vlog_stream, driver_env.fuzzing);
+                         driver_env.fs, options_.codegen_options.target,
+                         options);
   CARBON_VLOG_TO(driver_env.vlog_stream,
                  "*** Check::CheckParseTrees done ***\n");
   for (auto& unit : units) {

toolchain/driver/format_subcommand.cpp

@@ -82,7 +82,9 @@ auto FormatSubcommand::Run(DriverEnv& driver_env) -> DriverResult {
       continue;
     }
     SharedValueStores value_stores;
-    auto tokens = Lex::Lex(value_stores, *source, driver_env.consumer);
+    Lex::LexOptions lex_options;
+    lex_options.consumer = &driver_env.consumer;
+    auto tokens = Lex::Lex(value_stores, *source, lex_options);
 
     RawStringOstream buffer;
     if (Format::Format(tokens, buffer)) {

toolchain/language_server/context.cpp

@@ -131,10 +131,16 @@ auto Context::File::SetText(Context& context, std::optional<int64_t> version,
   }
   source_ = std::make_unique<SourceBuffer>(std::move(*source));
   value_stores_ = std::make_unique<SharedValueStores>();
+
+  Lex::LexOptions lex_options;
+  lex_options.consumer = &consumer;
   tokens_ = std::make_unique<Lex::TokenizedBuffer>(
-      Lex::Lex(*value_stores_, *source_, consumer));
-  tree_ = std::make_unique<Parse::Tree>(
-      Parse::Parse(*tokens_, consumer, context.vlog_stream()));
+      Lex::Lex(*value_stores_, *source_, lex_options));
+
+  Parse::ParseOptions parse_options;
+  parse_options.consumer = &consumer;
+  parse_options.vlog_stream = context.vlog_stream();
+  tree_ = std::make_unique<Parse::Tree>(Parse::Parse(*tokens_, parse_options));
   tree_and_subtrees_ =
       std::make_unique<Parse::TreeAndSubtrees>(*tokens_, *tree_);
 
@@ -153,11 +159,13 @@ auto Context::File::SetText(Context& context, std::optional<int64_t> version,
   };
   llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> fs =
       new llvm::vfs::InMemoryFileSystem;
+
   // TODO: Include the prelude.
+  Check::CheckParseTreesOptions check_options;
+  check_options.vlog_stream = context.vlog_stream();
   Check::CheckParseTrees(
-      units, llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>(getter),
-      /*prelude_import=*/false, fs, llvm::sys::getDefaultTargetTriple(),
-      context.vlog_stream(), /*fuzzing=*/false);
+      units, llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>(getter), fs,
+      llvm::sys::getDefaultTargetTriple(), check_options);
 
   // Note we need to publish diagnostics even when empty.
   // TODO: Consider caching previously published diagnostics and only publishing

toolchain/lex/lex.cpp

@@ -1694,8 +1694,10 @@ auto Lexer::DiagnoseAndFixMismatchedBrackets() -> void {
 }
 
 auto Lex(SharedValueStores& value_stores, SourceBuffer& source,
-         Diagnostics::Consumer& consumer) -> TokenizedBuffer {
-  return Lexer(value_stores, source, consumer).Lex();
+         LexOptions options) -> TokenizedBuffer {
+  auto* consumer =
+      options.consumer ? options.consumer : &Diagnostics::ConsoleConsumer();
+  return Lexer(value_stores, source, *consumer).Lex();
 }
 
 }  // namespace Carbon::Lex

toolchain/lex/lex.h

@@ -12,13 +12,21 @@
 
 namespace Carbon::Lex {
 
+struct LexOptions {
+  // Options must be set individually, not through initialization.
+  explicit LexOptions() = default;
+
+  // If set, a consumer for diagnostics. Otherwise, diagnostics go to stderr.
+  Diagnostics::Consumer* consumer = nullptr;
+};
+
 // Lexes a buffer of source code into a tokenized buffer.
 //
 // The provided source buffer must outlive any returned `TokenizedBuffer`
 // which will refer into the source.
 auto Lex(SharedValueStores& value_stores,
-         SourceBuffer& source [[clang::lifetimebound]],
-         Diagnostics::Consumer& consumer) -> TokenizedBuffer;
+         SourceBuffer& source [[clang::lifetimebound]], LexOptions options)
+    -> TokenizedBuffer;
 
 }  // namespace Carbon::Lex
 

toolchain/lex/tokenized_buffer_benchmark.cpp

@@ -217,13 +217,17 @@ class LexerBenchHelper {
 
   auto Lex() -> TokenizedBuffer {
     Diagnostics::Consumer& consumer = Diagnostics::NullConsumer();
-    return Lex::Lex(value_stores_, source_, consumer);
+    Lex::LexOptions options;
+    options.consumer = &consumer;
+    return Lex::Lex(value_stores_, source_, options);
   }
 
   auto DiagnoseErrors() -> std::string {
     RawStringOstream result;
     Diagnostics::StreamConsumer consumer(&result);
-    auto buffer = Lex::Lex(value_stores_, source_, consumer);
+    Lex::LexOptions options;
+    options.consumer = &consumer;
+    auto buffer = Lex::Lex(value_stores_, source_, options);
     consumer.Flush();
     CARBON_CHECK(buffer.has_errors(),
                  "Asked to diagnose errors but none found!");

toolchain/lex/tokenized_buffer_fuzzer.cpp

@@ -35,7 +35,9 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) {
       SourceBuffer::MakeFromFile(fs, TestFileName, Diagnostics::NullConsumer());
 
   SharedValueStores value_stores;
-  auto buffer = Lex::Lex(value_stores, *source, Diagnostics::NullConsumer());
+  Lex::LexOptions options;
+  options.consumer = &Diagnostics::NullConsumer();
+  auto buffer = Lex::Lex(value_stores, *source, options);
   if (buffer.has_errors()) {
     return 0;
   }

toolchain/lower/lower.cpp

@@ -15,15 +15,19 @@ namespace Carbon::Lower {
 auto LowerToLLVM(
     llvm::LLVMContext& llvm_context,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
-    llvm::raw_ostream* llvm_verifier_stream, bool want_debug_info,
     llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
-    llvm::StringRef module_name, const SemIR::File& sem_ir,
-    const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream)
+    const SemIR::File& sem_ir, const LowerToLLVMOptions& options)
     -> std::unique_ptr<llvm::Module> {
-  Context context(llvm_context, std::move(fs), want_debug_info,
-                  tree_and_subtrees_getters, module_name, vlog_stream);
-  context.GetFileContext(&sem_ir, inst_namer).LowerDefinitions();
-  return std::move(context).Finalize(llvm_verifier_stream);
+  Context context(llvm_context, std::move(fs), options.want_debug_info,
+                  tree_and_subtrees_getters, sem_ir.filename(),
+                  options.vlog_stream);
+
+  // TODO: Consider disabling instruction naming by default if we're not
+  // producing textual LLVM IR.
+  SemIR::InstNamer inst_namer(&sem_ir);
+  context.GetFileContext(&sem_ir, &inst_namer).LowerDefinitions();
+
+  return std::move(context).Finalize(options.llvm_verifier_stream);
 }
 
 }  // namespace Carbon::Lower

toolchain/lower/lower.h

@@ -14,17 +14,26 @@
 
 namespace Carbon::Lower {
 
+struct LowerToLLVMOptions {
+  // Options must be set individually, not through initialization.
+  explicit LowerToLLVMOptions() = default;
+
+  // If set, enables LLVM IR verification.
+  llvm::raw_ostream* llvm_verifier_stream = nullptr;
+
+  // Whether to include debug info in lowered output.
+  bool want_debug_info = false;
+
+  // If set, enables verbose output.
+  llvm::raw_ostream* vlog_stream = nullptr;
+};
+
 // Lowers SemIR to LLVM IR.
-//
-// `llvm_verifier_stream` should be non-null when verification is desired.
-// TODO: Switch to a struct for parameters.
 auto LowerToLLVM(
     llvm::LLVMContext& llvm_context,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
-    llvm::raw_ostream* llvm_verifier_stream, bool want_debug_info,
     llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
-    llvm::StringRef module_name, const SemIR::File& sem_ir,
-    const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream)
+    const SemIR::File& sem_ir, const LowerToLLVMOptions& options)
     -> std::unique_ptr<llvm::Module>;
 
 }  // namespace Carbon::Lower

toolchain/parse/parse.cpp

@@ -17,11 +17,13 @@ auto HandleInvalid(Context& context) -> void {
                context.PopState());
 }
 
-auto Parse(Lex::TokenizedBuffer& tokens, Diagnostics::Consumer& consumer,
-           llvm::raw_ostream* vlog_stream) -> Tree {
+auto Parse(Lex::TokenizedBuffer& tokens, ParseOptions options) -> Tree {
+  auto* consumer =
+      options.consumer ? options.consumer : &Diagnostics::ConsoleConsumer();
+
   // Delegate to the parser.
   Tree tree(tokens);
-  Context context(&tree, &tokens, &consumer, vlog_stream);
+  Context context(&tree, &tokens, consumer, options.vlog_stream);
   PrettyStackTraceFunction context_dumper(
       [&](llvm::raw_ostream& output) { context.PrintForStackDump(output); });
 

toolchain/parse/parse.h

@@ -12,11 +12,21 @@
 
 namespace Carbon::Parse {
 
+struct ParseOptions {
+  // Options must be set individually, not through initialization.
+  explicit ParseOptions() = default;
+
+  // If set, a consumer for diagnostics. Otherwise, diagnostics go to stderr.
+  Diagnostics::Consumer* consumer = nullptr;
+
+  // If set, enables verbose output.
+  llvm::raw_ostream* vlog_stream = nullptr;
+};
+
 // Parses the token buffer into a `Tree`.
 //
 // This is the factory function which is used to build parse trees.
-auto Parse(Lex::TokenizedBuffer& tokens, Diagnostics::Consumer& consumer,
-           llvm::raw_ostream* vlog_stream) -> Tree;
+auto Parse(Lex::TokenizedBuffer& tokens, ParseOptions options) -> Tree;
 
 }  // namespace Carbon::Parse
 

toolchain/parse/parse_fuzzer.cpp

@@ -33,14 +33,18 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) {
 
   // Lex the input.
   SharedValueStores value_stores;
-  auto tokens = Lex::Lex(value_stores, *source, Diagnostics::NullConsumer());
+  Lex::LexOptions lex_options;
+  lex_options.consumer = &Diagnostics::NullConsumer();
+  auto tokens = Lex::Lex(value_stores, *source, lex_options);
   if (tokens.has_errors()) {
     return 0;
   }
 
   // Now parse it into a tree. Note that parsing will (when asserts are enabled)
   // walk the entire tree to verify it so we don't have to do that here.
-  Parse::Parse(tokens, Diagnostics::NullConsumer(), /*vlog_stream=*/nullptr);
+  Parse::ParseOptions parse_options;
+  parse_options.consumer = &Diagnostics::NullConsumer();
+  Parse::Parse(tokens, parse_options);
   return 0;
 }
 

toolchain/parse/tree_test.cpp

@@ -170,7 +170,9 @@ TEST_F(TreeTest, HighRecursion) {
   Lex::TokenizedBuffer& tokens = compile_helper_.GetTokenizedBuffer(code);
   ASSERT_FALSE(tokens.has_errors());
   Testing::MockDiagnosticConsumer consumer;
-  Tree tree = Parse(tokens, consumer, /*vlog_stream=*/nullptr);
+  Parse::ParseOptions options;
+  options.consumer = &consumer;
+  Tree tree = Parse(tokens, options);
   EXPECT_FALSE(tree.has_errors());
 }
 

toolchain/testing/compile_helper.cpp

@@ -15,8 +15,10 @@ auto CompileHelper::GetTokenizedBuffer(llvm::StringRef text,
   auto& source = GetSourceBuffer(text);
 
   value_store_storage_.emplace_front();
-  token_storage_.push_front(Lex::Lex(value_store_storage_.front(), source,
-                                     consumer ? *consumer : consumer_));
+  Lex::LexOptions options;
+  options.consumer = consumer ? consumer : &consumer_;
+  token_storage_.push_front(
+      Lex::Lex(value_store_storage_.front(), source, options));
   return token_storage_.front();
 }
 
@@ -29,8 +31,9 @@ auto CompileHelper::GetTokenizedBufferWithSharedValueStore(
 
 auto CompileHelper::GetTree(llvm::StringRef text) -> Parse::Tree& {
   auto& tokens = GetTokenizedBuffer(text);
-  tree_storage_.push_front(Parse::Parse(tokens, consumer_,
-                                        /*vlog_stream=*/nullptr));
+  Parse::ParseOptions options;
+  options.consumer = &consumer_;
+  tree_storage_.push_front(Parse::Parse(tokens, options));
   return tree_storage_.front();
 }