diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index dd9129aff16e..d006700fb7a0 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -70,6 +70,7 @@ cc_test( ":runtimes_cache", "//common:all_llvm_targets", "//common:check", + "//common:error_test_helpers", "//common:ostream", "//common:raw_string_ostream", "//testing/base:capture_std_streams", diff --git a/toolchain/driver/clang_runner.cpp b/toolchain/driver/clang_runner.cpp index 34900de007d3..16bb4b64ec27 100644 --- a/toolchain/driver/clang_runner.cpp +++ b/toolchain/driver/clang_runner.cpp @@ -223,7 +223,7 @@ auto ClangRunner::Run(llvm::ArrayRef args, } auto ClangRunner::RunWithNoRuntimes(llvm::ArrayRef args, - bool enable_leaking) -> bool { + bool enable_leaking) -> ErrorOr { std::string target = ComputeClangTarget(args); return RunInternal(args, target, /*target_resource_dir_path=*/std::nullopt, /*libunwind_path=*/std::nullopt, @@ -236,7 +236,7 @@ auto ClangRunner::RunInternal( std::optional libunwind_path, std::optional libcxx_path, bool enable_leaking) - -> bool { + -> ErrorOr { llvm::BumpPtrAllocator alloc; // Handle special dispatch for CC1 commands as they don't use the driver and @@ -298,6 +298,14 @@ auto ClangRunner::RunInternal( llvm::SmallVector cstr_args = BuildCStrArgs(clang_path_.native(), prefix_args, args, alloc); + // Expand any response files in the arguments. + bool is_clang_cl_mode = clang::driver::IsClangCL( + clang::driver::getDriverMode(clang_path_.native(), cstr_args)); + if (llvm::Error error = clang::driver::expandResponseFiles( + cstr_args, is_clang_cl_mode, alloc, fs_.get())) { + return Error(llvm::toString(std::move(error))); + } + CARBON_VLOG("Running Clang driver with the following arguments:\n"); for (const char* cstr_arg : llvm::ArrayRef(cstr_args)) { CARBON_VLOG(" '{0}'\n", cstr_arg); diff --git a/toolchain/driver/clang_runner.h b/toolchain/driver/clang_runner.h index 3d9a820fa47e..a59a02917d5c 100644 --- a/toolchain/driver/clang_runner.h +++ b/toolchain/driver/clang_runner.h @@ -77,8 +77,8 @@ class ClangRunner : ToolRunnerBase { // both to use and incorporate into the cache. // // Returns an error only if unable to successfully run Clang with the - // arguments. If able to run Clang, no error is returned a bool indicating - // whether than Clang invocation succeeded is returned. + // arguments. If able to run Clang, no error is returned, and a bool + // indicating whether than Clang invocation succeeded is returned. auto Run(llvm::ArrayRef args, Runtimes::Cache& runtimes_cache, llvm::ThreadPoolInterface& runtimes_build_thread_pool, @@ -94,15 +94,14 @@ class ClangRunner : ToolRunnerBase { // Run Clang with the provided arguments and without any target runtimes. // + // Similar to `Run`, but omits any target runtimes. + // // This method can be used to avoid building target-dependent resources when // unnecessary, but not all Clang command lines will work correctly. // Specifically, compile-only commands will typically work, while linking will // not. - // - // This function simply returns true or false depending on whether Clang runs - // successfully, as it should display any needed error messages. auto RunWithNoRuntimes(llvm::ArrayRef args, - bool enable_leaking = false) -> bool; + bool enable_leaking = false) -> ErrorOr; private: friend class ClangRuntimesBuilderBase; @@ -112,7 +111,7 @@ class ClangRunner : ToolRunnerBase { std::optional target_resource_dir_path, std::optional libunwind_path, std::optional libcxx_path, - bool enable_leaking) -> bool; + bool enable_leaking) -> ErrorOr; // Returns the target-specific source files for the builtins runtime library. auto CollectBuiltinsSrcFiles(const llvm::Triple& target_triple) diff --git a/toolchain/driver/clang_runner_test.cpp b/toolchain/driver/clang_runner_test.cpp index 03daed08479f..8567c2752e9a 100644 --- a/toolchain/driver/clang_runner_test.cpp +++ b/toolchain/driver/clang_runner_test.cpp @@ -11,6 +11,7 @@ #include #include +#include "common/error_test_helpers.h" #include "common/ostream.h" #include "common/raw_string_ostream.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" @@ -29,6 +30,7 @@ namespace Carbon { namespace { using ::testing::HasSubstr; +using Testing::IsSuccess; using ::testing::StrEq; // NOLINTNEXTLINE(modernize-use-trailing-return-type): Macro based function. @@ -71,8 +73,10 @@ TEST_F(ClangRunnerTest, Version) { std::string out; std::string err; - EXPECT_TRUE(Testing::CallWithCapturedOutput( - out, err, [&] { return runner.RunWithNoRuntimes({"--version"}); })); + EXPECT_THAT( + Testing::CallWithCapturedOutput( + out, err, [&] { return runner.RunWithNoRuntimes({"--version"}); }), + IsSuccess(true)); // The arguments to Clang should be part of the verbose log. EXPECT_THAT(test_os.TakeStr(), HasSubstr("--version")); @@ -100,12 +104,13 @@ TEST_F(ClangRunnerTest, DashC) { ClangRunner runner(&install_paths_, vfs_, &verbose_out); std::string out; std::string err; - EXPECT_TRUE(Testing::CallWithCapturedOutput( - out, err, - [&] { - return runner.RunWithNoRuntimes( - {"-c", test_file.string(), "-o", test_output.string()}); - })) + EXPECT_THAT(Testing::CallWithCapturedOutput( + out, err, + [&] { + return runner.RunWithNoRuntimes( + {"-c", test_file.string(), "-o", test_output.string()}); + }), + IsSuccess(true)) << "Verbose output from runner:\n" << verbose_out.TakeStr() << "\n"; verbose_out.clear(); @@ -129,12 +134,13 @@ TEST_F(ClangRunnerTest, BuitinHeaders) { ClangRunner runner(&install_paths_, vfs_, &verbose_out); std::string out; std::string err; - EXPECT_TRUE(Testing::CallWithCapturedOutput( - out, err, - [&] { - return runner.RunWithNoRuntimes( - {"-c", test_file.string(), "-o", test_output.string()}); - })) + EXPECT_THAT(Testing::CallWithCapturedOutput( + out, err, + [&] { + return runner.RunWithNoRuntimes( + {"-c", test_file.string(), "-o", test_output.string()}); + }), + IsSuccess(true)) << "Verbose output from runner:\n" << verbose_out.TakeStr() << "\n"; verbose_out.clear(); @@ -156,12 +162,13 @@ TEST_F(ClangRunnerTest, CompileMultipleFiles) { ClangRunner runner(&install_paths_, vfs_, &verbose_out); std::string out; std::string err; - EXPECT_TRUE(Testing::CallWithCapturedOutput( - out, err, - [&] { - return runner.RunWithNoRuntimes( - {"-c", file.string(), "-o", output.string()}); - })) + EXPECT_THAT(Testing::CallWithCapturedOutput( + out, err, + [&] { + return runner.RunWithNoRuntimes( + {"-c", file.string(), "-o", output.string()}); + }), + IsSuccess(true)) << "Verbose output from runner:\n" << verbose_out.TakeStr() << "\n"; verbose_out.clear(); @@ -191,16 +198,18 @@ TEST_F(ClangRunnerTest, LinkCommandEcho) { ClangRunner runner(&install_paths_, vfs_, &verbose_out); std::string out; std::string err; - EXPECT_TRUE(Testing::CallWithCapturedOutput( - out, err, - [&] { - // Note that we use the target independent run command here because - // we're just getting the echo-ed output back. For this to actually - // link, we'd need to have the target-dependent resources, but those are - // expensive to build so we only want to test them once (above). - return runner.RunWithNoRuntimes( - {"-###", "-o", "binary", foo_file.string(), bar_file.string()}); - })) + EXPECT_THAT( + Testing::CallWithCapturedOutput( + out, err, + [&] { + // Note that we use the target independent run command here because + // we're just getting the echo-ed output back. For this to actually + // link, we'd need to have the target-dependent resources, but those + // are expensive to build so we only want to test them once (above). + return runner.RunWithNoRuntimes( + {"-###", "-o", "binary", foo_file.string(), bar_file.string()}); + }), + IsSuccess(true)) << "Verbose output from runner:\n" << verbose_out.TakeStr() << "\n"; verbose_out.clear(); @@ -216,5 +225,41 @@ TEST_F(ClangRunnerTest, LinkCommandEcho) { EXPECT_THAT(out, StrEq("")); } +TEST_F(ClangRunnerTest, ParamsFile) { + // Use an overlay file system to ensure the params file expansion goes through + // the VFS. + llvm::IntrusiveRefCntPtr overlay_fs( + new llvm::vfs::OverlayFileSystem(vfs_)); + llvm::IntrusiveRefCntPtr in_memory_fs( + new llvm::vfs::InMemoryFileSystem); + overlay_fs->pushOverlay(in_memory_fs); + + std::filesystem::path params_path = "/params"; + in_memory_fs->addFile(params_path.native(), 0, + llvm::MemoryBuffer::getMemBuffer(R"( +--version +)")); + + RawStringOstream verbose_out; + ClangRunner runner(&install_paths_, overlay_fs, &verbose_out); + + std::string out; + std::string err; + EXPECT_THAT( + Testing::CallWithCapturedOutput(out, err, + [&] { + return runner.RunWithNoRuntimes( + {"@" + params_path.native()}); + }), + IsSuccess(true)) + << "Verbose output:\n" + << verbose_out.TakeStr(); + verbose_out.clear(); + + // Check that the version is printed, as if we directly passed `--version`. + EXPECT_THAT(err, StrEq("")); + EXPECT_THAT(out, HasSubstr("clang version")); +} + } // namespace } // namespace Carbon diff --git a/toolchain/driver/clang_runtimes.cpp b/toolchain/driver/clang_runtimes.cpp index 3bf52f40a89c..b555ea86e383 100644 --- a/toolchain/driver/clang_runtimes.cpp +++ b/toolchain/driver/clang_runtimes.cpp @@ -196,7 +196,9 @@ auto ClangRuntimesBuilderBase::ArchiveBuilder::CompileMember( obj_path.native(), src_path.native(), }); - if (!builder_->clang_->RunWithNoRuntimes(args)) { + CARBON_ASSIGN_OR_RETURN(bool success, + builder_->clang_->RunWithNoRuntimes(args)); + if (!success) { return Error( llvm::formatv("Failed to compile runtime source file '{0}'", src_file)); } @@ -583,21 +585,21 @@ auto ClangResourceDirBuilder::BuildCrtFile(llvm::StringRef src_file) installation().runtimes_root() / std::string_view(src_file); CARBON_VLOG("Building `{0}' from `{1}`...\n", out_path, src_path); - bool success = clang_->RunWithNoRuntimes({ - "-no-canonical-prefixes", - "-DCRT_HAS_INITFINI_ARRAY", - "-DEH_USE_FRAME_REGISTRY", - "-O3", - "-fPIC", - "-ffreestanding", - "-std=c11", - "-w", - "-c", - target_flag_, - "-o", - out_path.native(), - src_path.native(), - }); + CARBON_ASSIGN_OR_RETURN(bool success, clang_->RunWithNoRuntimes({ + "-no-canonical-prefixes", + "-DCRT_HAS_INITFINI_ARRAY", + "-DEH_USE_FRAME_REGISTRY", + "-O3", + "-fPIC", + "-ffreestanding", + "-std=c11", + "-w", + "-c", + target_flag_, + "-o", + out_path.native(), + src_path.native(), + })); if (success) { return Success(); diff --git a/toolchain/driver/lld_runner_test.cpp b/toolchain/driver/lld_runner_test.cpp index ad07ba1cbde7..c576a70723e7 100644 --- a/toolchain/driver/lld_runner_test.cpp +++ b/toolchain/driver/lld_runner_test.cpp @@ -89,7 +89,7 @@ static auto CompileTwoSources(const InstallPaths& install_paths, std::string target_arg = llvm::formatv("--target={0}", target).str(); std::string out; std::string err; - CARBON_CHECK(Testing::CallWithCapturedOutput( + CARBON_CHECK(*Testing::CallWithCapturedOutput( out, err, [&] { auto run_result = clang.RunWithNoRuntimes( @@ -101,7 +101,7 @@ static auto CompileTwoSources(const InstallPaths& install_paths, verbose_out.TakeStr(), err); verbose_out.clear(); - CARBON_CHECK(Testing::CallWithCapturedOutput( + CARBON_CHECK(*Testing::CallWithCapturedOutput( out, err, [&] { auto run_result = clang.RunWithNoRuntimes(