From 26e58b4587ad01cae09406650bf0325bee7f7265 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Mon, 4 Nov 2024 14:42:57 -0800 Subject: [PATCH] Refactor subcommand addition for sharing. (#4474) Trying to standardize the setup a little more. --- toolchain/driver/clang_subcommand.cpp | 24 +++++++------ toolchain/driver/clang_subcommand.h | 8 +++-- toolchain/driver/compile_subcommand.cpp | 32 +++++++++-------- toolchain/driver/compile_subcommand.h | 8 +++-- toolchain/driver/driver.cpp | 36 +++++-------------- toolchain/driver/driver_subcommand.h | 22 ++++++++++++ toolchain/driver/format_subcommand.cpp | 16 +++++---- toolchain/driver/format_subcommand.h | 8 +++-- .../driver/language_server_subcommand.cpp | 5 ++- toolchain/driver/language_server_subcommand.h | 4 ++- toolchain/driver/link_subcommand.cpp | 26 +++++++------- toolchain/driver/link_subcommand.h | 8 +++-- 12 files changed, 110 insertions(+), 87 deletions(-) diff --git a/toolchain/driver/clang_subcommand.cpp b/toolchain/driver/clang_subcommand.cpp index 2d1b89cc78677..7165331ca2612 100644 --- a/toolchain/driver/clang_subcommand.cpp +++ b/toolchain/driver/clang_subcommand.cpp @@ -9,7 +9,18 @@ namespace Carbon { -constexpr CommandLine::CommandInfo ClangOptions::Info = { +auto ClangOptions::Build(CommandLine::CommandBuilder& b) -> void { + b.AddStringPositionalArg( + { + .name = "ARG", + .help = R"""( +Arguments passed to Clang. +)""", + }, + [&](auto& arg_b) { arg_b.Append(&args); }); +} + +static constexpr CommandLine::CommandInfo SubcommandInfo = { .name = "clang", .help = R"""( Runs Clang on arguments. @@ -27,16 +38,7 @@ results in an indirect Clang invocation. )""", }; -auto ClangOptions::Build(CommandLine::CommandBuilder& b) -> void { - b.AddStringPositionalArg( - { - .name = "ARG", - .help = R"""( -Arguments passed to Clang. -)""", - }, - [&](auto& arg_b) { arg_b.Append(&args); }); -} +ClangSubcommand::ClangSubcommand() : DriverSubcommand(SubcommandInfo) {} // TODO: This lacks a lot of features from the main driver code. We may need to // add more. diff --git a/toolchain/driver/clang_subcommand.h b/toolchain/driver/clang_subcommand.h index 595bfb016f63d..6d3f9c5e77848 100644 --- a/toolchain/driver/clang_subcommand.h +++ b/toolchain/driver/clang_subcommand.h @@ -17,8 +17,6 @@ namespace Carbon { // // See the implementation of `Build` for documentation on members. struct ClangOptions { - static const CommandLine::CommandInfo Info; - auto Build(CommandLine::CommandBuilder& b) -> void; llvm::SmallVector args; @@ -27,7 +25,11 @@ struct ClangOptions { // Implements the clang subcommand of the driver. class ClangSubcommand : public DriverSubcommand { public: - auto BuildOptions(CommandLine::CommandBuilder& b) { options_.Build(b); } + explicit ClangSubcommand(); + + auto BuildOptions(CommandLine::CommandBuilder& b) -> void override { + options_.Build(b); + } auto Run(DriverEnv& driver_env) -> DriverResult override; diff --git a/toolchain/driver/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp index 79b4d93b68bc1..d399025a37a2c 100644 --- a/toolchain/driver/compile_subcommand.cpp +++ b/toolchain/driver/compile_subcommand.cpp @@ -42,21 +42,6 @@ auto operator<<(llvm::raw_ostream& out, CompileOptions::Phase phase) return out; } -constexpr CommandLine::CommandInfo CompileOptions::Info = { - .name = "compile", - .help = R"""( -Compile Carbon source code. - -This subcommand runs the Carbon compiler over input source code, checking it for -errors and producing the requested output. - -Error messages are written to the standard error stream. - -Different phases of the compiler can be selected to run, and intermediate state -can be written to standard output as these phases progress. -)""", -}; - auto CompileOptions::Build(CommandLine::CommandBuilder& b) -> void { b.AddStringPositionalArg( { @@ -284,6 +269,23 @@ Emit DWARF debug information. }); } +static constexpr CommandLine::CommandInfo SubcommandInfo = { + .name = "compile", + .help = R"""( +Compile Carbon source code. + +This subcommand runs the Carbon compiler over input source code, checking it for +errors and producing the requested output. + +Error messages are written to the standard error stream. + +Different phases of the compiler can be selected to run, and intermediate state +can be written to standard output as these phases progress. +)""", +}; + +CompileSubcommand::CompileSubcommand() : DriverSubcommand(SubcommandInfo) {} + auto CompileSubcommand::ValidateOptions(DriverEnv& driver_env) const -> bool { using Phase = CompileOptions::Phase; switch (options_.phase) { diff --git a/toolchain/driver/compile_subcommand.h b/toolchain/driver/compile_subcommand.h index fce7052434bd5..c96f0c5878283 100644 --- a/toolchain/driver/compile_subcommand.h +++ b/toolchain/driver/compile_subcommand.h @@ -19,8 +19,6 @@ namespace Carbon { // // See the implementation of `Build` for documentation on members. struct CompileOptions { - static const CommandLine::CommandInfo Info; - enum class Phase : int8_t { Lex, Parse, @@ -65,7 +63,11 @@ struct CompileOptions { // Implements the compile subcommand of the driver. class CompileSubcommand : public DriverSubcommand { public: - auto BuildOptions(CommandLine::CommandBuilder& b) { options_.Build(b); } + explicit CompileSubcommand(); + + auto BuildOptions(CommandLine::CommandBuilder& b) -> void override { + options_.Build(b); + } auto Run(DriverEnv& driver_env) -> DriverResult override; diff --git a/toolchain/driver/driver.cpp b/toolchain/driver/driver.cpp index 309b2b6cc2711..5022df6bf8663 100644 --- a/toolchain/driver/driver.cpp +++ b/toolchain/driver/driver.cpp @@ -34,7 +34,7 @@ struct Options { LinkSubcommand link; // On success, this is set to the subcommand to run. - DriverSubcommand* subcommand = nullptr; + DriverSubcommand* selected_subcommand = nullptr; }; } // namespace @@ -73,31 +73,11 @@ auto Options::Build(CommandLine::CommandBuilder& b) -> void { }, [&](CommandLine::FlagBuilder& arg_b) { arg_b.Set(&fuzzing); }); - b.AddSubcommand(ClangOptions::Info, [&](CommandLine::CommandBuilder& sub_b) { - clang.BuildOptions(sub_b); - sub_b.Do([&] { subcommand = &clang; }); - }); - - b.AddSubcommand(CompileOptions::Info, - [&](CommandLine::CommandBuilder& sub_b) { - compile.BuildOptions(sub_b); - sub_b.Do([&] { subcommand = &compile; }); - }); - - b.AddSubcommand(FormatOptions::Info, [&](CommandLine::CommandBuilder& sub_b) { - format.BuildOptions(sub_b); - sub_b.Do([&] { subcommand = &format; }); - }); - - b.AddSubcommand(LanguageServerSubcommand::Info, - [&](CommandLine::CommandBuilder& sub_b) { - sub_b.Do([&] { subcommand = &language_server; }); - }); - - b.AddSubcommand(LinkOptions::Info, [&](CommandLine::CommandBuilder& sub_b) { - link.BuildOptions(sub_b); - sub_b.Do([&] { subcommand = &link; }); - }); + clang.AddTo(b, &selected_subcommand); + compile.AddTo(b, &selected_subcommand); + format.AddTo(b, &selected_subcommand); + language_server.AddTo(b, &selected_subcommand); + link.AddTo(b, &selected_subcommand); b.RequiresSubcommand(); } @@ -128,8 +108,8 @@ auto Driver::RunCommand(llvm::ArrayRef args) -> DriverResult { SetFuzzing(); } - CARBON_CHECK(options.subcommand != nullptr); - return options.subcommand->Run(driver_env_); + CARBON_CHECK(options.selected_subcommand != nullptr); + return options.selected_subcommand->Run(driver_env_); } auto Driver::SetFuzzing() -> void { driver_env_.fuzzing = true; } diff --git a/toolchain/driver/driver_subcommand.h b/toolchain/driver/driver_subcommand.h index 4ef54615895ab..de31584234d04 100644 --- a/toolchain/driver/driver_subcommand.h +++ b/toolchain/driver/driver_subcommand.h @@ -5,6 +5,7 @@ #ifndef CARBON_TOOLCHAIN_DRIVER_DRIVER_SUBCOMMAND_H_ #define CARBON_TOOLCHAIN_DRIVER_DRIVER_SUBCOMMAND_H_ +#include "common/command_line.h" #include "common/ostream.h" #include "llvm/Support/VirtualFileSystem.h" #include "toolchain/driver/driver_env.h" @@ -25,7 +26,28 @@ struct DriverResult { // A subcommand for the driver. class DriverSubcommand { public: + explicit DriverSubcommand(CommandLine::CommandInfo info) : info_(info) {} + + // Adds the subcommand to the main command, assigning `selected_command` when + // the subcommand is in use. + auto AddTo(CommandLine::CommandBuilder& b, + DriverSubcommand** selected_subcommand) -> void { + b.AddSubcommand(info_, [this, selected_subcommand]( + CommandLine::CommandBuilder& sub_b) { + BuildOptions(sub_b); + sub_b.Do([this, selected_subcommand] { *selected_subcommand = this; }); + }); + } + + // Adds command line options. + virtual auto BuildOptions(CommandLine::CommandBuilder& b) -> void = 0; + + // Runs the command. virtual auto Run(DriverEnv& driver_env) -> DriverResult = 0; + + private: + // Subcommand information. + CommandLine::CommandInfo info_; }; } // namespace Carbon diff --git a/toolchain/driver/format_subcommand.cpp b/toolchain/driver/format_subcommand.cpp index c99addf0138bb..6ec25096b95fc 100644 --- a/toolchain/driver/format_subcommand.cpp +++ b/toolchain/driver/format_subcommand.cpp @@ -14,13 +14,6 @@ namespace Carbon { -constexpr CommandLine::CommandInfo FormatOptions::Info = { - .name = "format", - .help = R"""( -Format Carbon source code. -)""", -}; - auto FormatOptions::Build(CommandLine::CommandBuilder& b) -> void { b.AddStringPositionalArg( { @@ -49,6 +42,15 @@ Not valid when multiple files are passed for formatting. [&](auto& arg_b) { arg_b.Set(&output_filename); }); } +static constexpr CommandLine::CommandInfo SubcommandInfo = { + .name = "format", + .help = R"""( +Format Carbon source code. +)""", +}; + +FormatSubcommand::FormatSubcommand() : DriverSubcommand(SubcommandInfo) {} + auto FormatSubcommand::Run(DriverEnv& driver_env) -> DriverResult { DriverResult result = {.success = true}; if (options_.input_filenames.size() > 1 && diff --git a/toolchain/driver/format_subcommand.h b/toolchain/driver/format_subcommand.h index 9d0cba20d6202..9d07b7d8da41c 100644 --- a/toolchain/driver/format_subcommand.h +++ b/toolchain/driver/format_subcommand.h @@ -15,8 +15,6 @@ namespace Carbon { // // See the implementation of `Build` for documentation on members. struct FormatOptions { - static const CommandLine::CommandInfo Info; - auto Build(CommandLine::CommandBuilder& b) -> void; llvm::StringRef output_filename; @@ -26,7 +24,11 @@ struct FormatOptions { // Implements the format subcommand of the driver. class FormatSubcommand : public DriverSubcommand { public: - auto BuildOptions(CommandLine::CommandBuilder& b) { options_.Build(b); } + explicit FormatSubcommand(); + + auto BuildOptions(CommandLine::CommandBuilder& b) -> void override { + options_.Build(b); + } auto Run(DriverEnv& driver_env) -> DriverResult override; diff --git a/toolchain/driver/language_server_subcommand.cpp b/toolchain/driver/language_server_subcommand.cpp index f01853452a8d1..b7ddab1bd8ba6 100644 --- a/toolchain/driver/language_server_subcommand.cpp +++ b/toolchain/driver/language_server_subcommand.cpp @@ -8,13 +8,16 @@ namespace Carbon { -constexpr CommandLine::CommandInfo LanguageServerSubcommand::Info = { +static constexpr CommandLine::CommandInfo SubcommandInfo = { .name = "language-server", .help = R"""( Runs the language server. )""", }; +LanguageServerSubcommand::LanguageServerSubcommand() + : DriverSubcommand(SubcommandInfo) {} + auto LanguageServerSubcommand::Run(DriverEnv& driver_env) -> DriverResult { // TODO: Consider a way to override stdin, but it's a `FILE*` so less // convenient to work with. diff --git a/toolchain/driver/language_server_subcommand.h b/toolchain/driver/language_server_subcommand.h index 5cb85c613e81a..73b1e6e5769a3 100644 --- a/toolchain/driver/language_server_subcommand.h +++ b/toolchain/driver/language_server_subcommand.h @@ -17,7 +17,9 @@ namespace Carbon { // Implements the link subcommand of the driver. class LanguageServerSubcommand : public DriverSubcommand { public: - static const CommandLine::CommandInfo Info; + explicit LanguageServerSubcommand(); + + auto BuildOptions(CommandLine::CommandBuilder& /*b*/) -> void override {} auto Run(DriverEnv& driver_env) -> DriverResult override; }; diff --git a/toolchain/driver/link_subcommand.cpp b/toolchain/driver/link_subcommand.cpp index 85d94bc5b1a90..043f5edc42c0f 100644 --- a/toolchain/driver/link_subcommand.cpp +++ b/toolchain/driver/link_subcommand.cpp @@ -9,18 +9,6 @@ namespace Carbon { -constexpr CommandLine::CommandInfo LinkOptions::Info = { - .name = "link", - .help = R"""( -Link Carbon executables. - -This subcommand links Carbon executables by combining object files. - -TODO: Support linking binary libraries, both archives and shared libraries. -TODO: Support linking against binary libraries. -)""", -}; - auto LinkOptions::Build(CommandLine::CommandBuilder& b) -> void { b.AddStringPositionalArg( { @@ -80,6 +68,20 @@ static void AddOSFlags(llvm::StringRef target, } } +static constexpr CommandLine::CommandInfo SubcommandInfo = { + .name = "link", + .help = R"""( +Link Carbon executables. + +This subcommand links Carbon executables by combining object files. + +TODO: Support linking binary libraries, both archives and shared libraries. +TODO: Support linking against binary libraries. +)""", +}; + +LinkSubcommand::LinkSubcommand() : DriverSubcommand(SubcommandInfo) {} + auto LinkSubcommand::Run(DriverEnv& driver_env) -> DriverResult { // TODO: Currently we use the Clang driver to link. This works well on Unix // OSes but we likely need to directly build logic to invoke `link.exe` on diff --git a/toolchain/driver/link_subcommand.h b/toolchain/driver/link_subcommand.h index 3a4013fa03b54..5eb127629b073 100644 --- a/toolchain/driver/link_subcommand.h +++ b/toolchain/driver/link_subcommand.h @@ -18,8 +18,6 @@ namespace Carbon { // // See the implementation of `Build` for documentation on members. struct LinkOptions { - static const CommandLine::CommandInfo Info; - auto Build(CommandLine::CommandBuilder& b) -> void; CodegenOptions codegen_options; @@ -30,7 +28,11 @@ struct LinkOptions { // Implements the link subcommand of the driver. class LinkSubcommand : public DriverSubcommand { public: - auto BuildOptions(CommandLine::CommandBuilder& b) { options_.Build(b); } + explicit LinkSubcommand(); + + auto BuildOptions(CommandLine::CommandBuilder& b) -> void override { + options_.Build(b); + } auto Run(DriverEnv& driver_env) -> DriverResult override;