diff --git a/toolchain/base/BUILD b/toolchain/base/BUILD index 20ee253e74e4..6c6be345a6e9 100644 --- a/toolchain/base/BUILD +++ b/toolchain/base/BUILD @@ -53,7 +53,9 @@ cc_library( srcs = ["clang_invocation.cpp"], hdrs = ["clang_invocation.h"], deps = [ + ":install_paths", "//common:check", + "//common:string_helpers", "//toolchain/diagnostics:diagnostic_emitter", "@llvm-project//clang:basic", "@llvm-project//clang:frontend", diff --git a/toolchain/base/clang_invocation.cpp b/toolchain/base/clang_invocation.cpp index a424a91df3ba..35ea4cd02dd3 100644 --- a/toolchain/base/clang_invocation.cpp +++ b/toolchain/base/clang_invocation.cpp @@ -4,8 +4,16 @@ #include "toolchain/base/clang_invocation.h" +#include +#include + #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/Utils.h" +#include "common/string_helpers.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/FormatVariadic.h" namespace Carbon { @@ -60,23 +68,36 @@ class ClangDriverDiagnosticConsumer : public clang::DiagnosticConsumer { } // namespace -static auto BuildClangInvocationImpl( - Diagnostics::NoLocEmitter& emitter, - llvm::IntrusiveRefCntPtr fs, - llvm::ArrayRef clang_path_and_args) +auto BuildClangInvocation(Diagnostics::Consumer& consumer, + llvm::IntrusiveRefCntPtr fs, + const InstallPaths& install_paths, + llvm::StringRef target_str, + llvm::ArrayRef extra_args) -> std::unique_ptr { + Diagnostics::ErrorTrackingConsumer error_tracker(consumer); + Diagnostics::NoLocEmitter emitter(&error_tracker); + ClangDriverDiagnosticConsumer diagnostics_consumer(&emitter); + llvm::SmallVector args; + args.push_back("--start-no-unused-arguments"); + AppendDefaultClangArgs(install_paths, target_str, args); + args.push_back("--end-no-unused-arguments"); + args.append({ + llvm::formatv("--target={0}", target_str).str(), + + // Add our include file name as the input file, and force it to be + // interpreted as C++. + "-x", + "c++", + IncludesFileName, + }); + // The clang driver inconveniently wants an array of `const char*`, so convert // the arguments. - llvm::SmallVector driver_args(llvm::map_range( - clang_path_and_args, [](const std::string& str) { return str.c_str(); })); - - // Add our include file name as the input file, and force it to be interpreted - // as C++. - driver_args.push_back("-x"); - driver_args.push_back("c++"); - driver_args.push_back(IncludesFileName); + llvm::OwningArrayRef cstr_arg_storage; + llvm::SmallVector cstr_args = BuildCStrArgs( + install_paths.clang_path().native(), args, extra_args, cstr_arg_storage); // Build a diagnostics engine. Note that we don't have any diagnostic options // yet; they're produced by running the driver. @@ -88,19 +109,8 @@ static auto BuildClangInvocationImpl( // Ask the driver to process the arguments and build a corresponding clang // frontend invocation. - return clang::createInvocation(driver_args, - {.Diags = driver_diags, .VFS = fs}); -} - -auto BuildClangInvocation(Diagnostics::Consumer& consumer, - llvm::IntrusiveRefCntPtr fs, - llvm::ArrayRef clang_path_and_args) - -> std::unique_ptr { - Diagnostics::ErrorTrackingConsumer error_tracker(consumer); - Diagnostics::NoLocEmitter emitter(&error_tracker); - - // Forward to the implementation to avoid exposing `import_cpp` outside check. - auto invocation = BuildClangInvocationImpl(emitter, fs, clang_path_and_args); + auto invocation = + clang::createInvocation(cstr_args, {.Diags = driver_diags, .VFS = fs}); // If Clang produced an error, throw away its invocation. if (error_tracker.seen_error()) { @@ -110,4 +120,49 @@ auto BuildClangInvocation(Diagnostics::Consumer& consumer, return invocation; } +auto AppendDefaultClangArgs(const InstallPaths& /*install_paths*/, + llvm::StringRef target_str, + llvm::SmallVectorImpl& args) -> void { + args.append({ + // Enable PIE by default, but allow it to be overridden by Clang + // arguments. Clang's default is configurable, but we'd like our + // defaults to be more stable. + // TODO: Decide if we want this. + "-fPIE", + + // Override the default linker to use. + "-fuse-ld=lld", + }); + + // Add target-specific flags. + llvm::Triple triple(target_str); + switch (triple.getOS()) { + case llvm::Triple::Darwin: + case llvm::Triple::MacOSX: + // On macOS we need to set the sysroot to a viable SDK. Currently, this + // hard codes the path to be the unversioned symlink. The prefix is also + // hard coded in Homebrew and so this seems likely to work reasonably + // well. Homebrew and I suspect the Xcode Clang both have this hard coded + // at build time, so this seems reasonably safe but we can revisit if/when + // needed. + args.push_back( + "--sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk"); + + // We also need to insist on a modern linker, otherwise the driver tries + // too old and deprecated flags. The specific number here comes from an + // inspection of the Clang driver source code to understand where features + // were enabled, and this appears to be the latest version to control + // driver behavior. + // + // TODO: We should replace this with use of `lld` eventually. + args.push_back("-mlinker-version=705"); + break; + + default: + break; + } + + // TODO: Add flags for the installed runtimes using `install_paths`. +} + } // namespace Carbon diff --git a/toolchain/base/clang_invocation.h b/toolchain/base/clang_invocation.h index bae2b58a7557..e81867f241cb 100644 --- a/toolchain/base/clang_invocation.h +++ b/toolchain/base/clang_invocation.h @@ -11,6 +11,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/Support/VirtualFileSystem.h" +#include "toolchain/base/install_paths.h" #include "toolchain/diagnostics/diagnostic_emitter.h" namespace Carbon { @@ -20,9 +21,20 @@ namespace Carbon { // `consumer` if the arguments are invalid. auto BuildClangInvocation(Diagnostics::Consumer& consumer, llvm::IntrusiveRefCntPtr fs, - llvm::ArrayRef clang_path_and_args) + const InstallPaths& install_paths, + llvm::StringRef target_str, + llvm::ArrayRef extra_args = {}) -> std::unique_ptr; +// Appends the default Clang command line arguments used when building a +// Carbon-compatible Clang invocation. +// +// Where possible, code should use `BuildClangInvocation` above. However, when +// invoking Clang directly, this can be used to get the core compatible flags. +auto AppendDefaultClangArgs(const InstallPaths& install_paths, + llvm::StringRef target_str, + llvm::SmallVectorImpl& args) -> void; + } // namespace Carbon #endif // CARBON_TOOLCHAIN_BASE_CLANG_INVOCATION_H_ diff --git a/toolchain/check/testdata/interop/cpp/cpp_namespace.carbon b/toolchain/check/testdata/interop/cpp/cpp_namespace.carbon index c32ef165ca66..fe5684ce945d 100644 --- a/toolchain/check/testdata/interop/cpp/cpp_namespace.carbon +++ b/toolchain/check/testdata/interop/cpp/cpp_namespace.carbon @@ -4,6 +4,8 @@ // // INCLUDE-FILE: toolchain/testing/testdata/min_prelude/none.carbon // +// EXTRA-ARGS: --target=aarch64-unknown-linux +// // AUTOUPDATE // TIP: To test this file alone, run: // TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/interop/cpp/cpp_namespace.carbon diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index eac5afdd2b96..1812694c969f 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -39,6 +39,7 @@ cc_library( "//common:string_helpers", "//common:vlog", "//third_party/llvm:clang_cc1", + "//toolchain/base:clang_invocation", "//toolchain/base:install_paths", "//toolchain/base:kind_switch", "//toolchain/base:runtime_sources", diff --git a/toolchain/driver/clang_runner.cpp b/toolchain/driver/clang_runner.cpp index 5ac2163d6ba0..b9025d013d66 100644 --- a/toolchain/driver/clang_runner.cpp +++ b/toolchain/driver/clang_runner.cpp @@ -51,6 +51,7 @@ #include "llvm/Support/raw_ostream.h" #include "llvm/TargetParser/Host.h" #include "third_party/llvm/clang_cc1.h" +#include "toolchain/base/clang_invocation.h" #include "toolchain/base/install_paths.h" #include "toolchain/driver/clang_runtimes.h" #include "toolchain/driver/runtimes_cache.h" @@ -214,12 +215,12 @@ auto ClangRunner::RunInternal( // Rebuild the args as C-string args. llvm::OwningArrayRef cstr_arg_storage; - llvm::SmallVector cstr_args = - BuildCStrArgs(clang_path, args, cstr_arg_storage); // Handle special dispatch for CC1 commands as they don't use the driver and // we don't synthesize any default arguments there. if (!args.empty() && args[0].starts_with("-cc1")) { + llvm::SmallVector cstr_args = + BuildCStrArgs(clang_path, args, cstr_arg_storage); if (args[0] == "-cc1") { CARBON_VLOG("Dispatching `-cc1` command line..."); int exit_code = @@ -246,6 +247,20 @@ auto ClangRunner::RunInternal( return exit_code == 0; } + // We start with a custom prefix of arguments to establish Carbon's default + // configuration for invoking Clang. These may not all be needed for all + // invocations, so we also suppress warnings about any that are ignored. + llvm::SmallVector prefix_args; + prefix_args.push_back("--start-no-unused-arguments"); + + AppendDefaultClangArgs(*installation_, target, prefix_args); + + prefix_args.push_back("--end-no-unused-arguments"); + + // Rebuild the args as C-string args. + llvm::SmallVector cstr_args = + BuildCStrArgs(clang_path, prefix_args, args, cstr_arg_storage); + 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/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp index 377a0a864c17..48faae45428e 100644 --- a/toolchain/driver/compile_subcommand.cpp +++ b/toolchain/driver/compile_subcommand.cpp @@ -1082,31 +1082,25 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { // TODO: Share any arguments we specify here with the `carbon clang` // subcommand. { - llvm::SmallVector clang_path_and_args = { - driver_env.installation->clang_path(), - // Propagate the target to Clang. - llvm::formatv("--target={0}", options_.codegen_options.target).str(), - // Enable PIE by default, but allow it to be overridden by Clang - // arguments. Clang's default is configurable, but we'd like our - // defaults to be more stable. - // TODO: Decide if we want this. - "-fPIE", - // Propagate our optimization level to Clang as a default. This can be - // overridden by Clang arguments, but doing so will only have an effect - // if those arguments affect Clang's IR, not its pass pipeline. - GetClangOptimizationFlag(options_.opt_level).str(), - }; if (driver_env.fuzzing && !options_.clang_args.empty()) { // Parsing specific Clang arguments can reach deep into // external libraries that aren't fuzz clean. TestAndDiagnoseIfFuzzingExternalLibraries(driver_env, "compile"); return {.success = false}; } - for (auto str : options_.clang_args) { - clang_path_and_args.push_back(str.str()); - } - clang_invocation = BuildClangInvocation(driver_env.consumer, driver_env.fs, - clang_path_and_args); + + // TODO: Move this into `BuildClangInvocation` when it can accept an + // optimization level. + llvm::SmallVector clang_args = { + // Propagate our optimization level to Clang as a default. This can be + // overridden by Clang arguments, but doing so will only have an effect + // if those arguments affect Clang's IR, not its pass pipeline. + GetClangOptimizationFlag(options_.opt_level), + }; + clang_args.append(options_.clang_args); + clang_invocation = BuildClangInvocation( + driver_env.consumer, driver_env.fs, *driver_env.installation, + options_.codegen_options.target, clang_args); if (!clang_invocation) { return {.success = false}; } diff --git a/toolchain/driver/link_subcommand.cpp b/toolchain/driver/link_subcommand.cpp index 04c3ddd4741a..315ad3abc47c 100644 --- a/toolchain/driver/link_subcommand.cpp +++ b/toolchain/driver/link_subcommand.cpp @@ -38,36 +38,6 @@ The linked file name. The output is always a linked binary. codegen_options.Build(b); } -static void AddOSFlags(llvm::StringRef target, - llvm::SmallVectorImpl& args) { - llvm::Triple triple(target); - switch (triple.getOS()) { - case llvm::Triple::Darwin: - case llvm::Triple::MacOSX: - // On macOS we need to set the sysroot to a viable SDK. Currently, this - // hard codes the path to be the unversioned symlink. The prefix is also - // hard coded in Homebrew and so this seems likely to work reasonably - // well. Homebrew and I suspect the Xcode Clang both have this hard coded - // at build time, so this seems reasonably safe but we can revisit if/when - // needed. - args.push_back( - "--sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk"); - // We also need to insist on a modern linker, otherwise the driver tries - // too old and deprecated flags. The specific number here comes from an - // inspection of the Clang driver source code to understand where features - // were enabled, and this appears to be the latest version to control - // driver behavior. - // - // TODO: We should replace this with use of `lld` eventually. - args.push_back("-mlinker-version=705"); - break; - - default: - // By default, just let the Clang driver handle everything. - break; - } -} - static constexpr CommandLine::CommandInfo SubcommandInfo = { .name = "link", .help = R"""( @@ -99,9 +69,6 @@ auto LinkSubcommand::Run(DriverEnv& driver_env) -> DriverResult { llvm::formatv("--target={0}", options_.codegen_options.target).str(); clang_args.push_back(target_arg); - // Use LLD, which we provide in our install directory, for linking. - clang_args.push_back("-fuse-ld=lld"); - // Disable linking the C++ standard library until can build and ship it as // part of the Carbon toolchain. This clearly won't work once we get into // interop, but for now it avoids spurious failures and distraction. The plan @@ -110,9 +77,6 @@ auto LinkSubcommand::Run(DriverEnv& driver_env) -> DriverResult { // TODO: Replace this when ready. clang_args.push_back("-nostdlib++"); - // Add OS-specific flags based on the target. - AddOSFlags(options_.codegen_options.target, clang_args); - clang_args.push_back("-o"); clang_args.push_back(options_.output_filename); clang_args.append(options_.object_filenames.begin(), diff --git a/toolchain/driver/testdata/compile/fail_clang_arg_extra_input.carbon b/toolchain/driver/testdata/compile/fail_clang_arg_extra_input.carbon index d9fdaed7a9b9..b32e61efaaef 100644 --- a/toolchain/driver/testdata/compile/fail_clang_arg_extra_input.carbon +++ b/toolchain/driver/testdata/compile/fail_clang_arg_extra_input.carbon @@ -9,7 +9,7 @@ // TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/compile/fail_clang_arg_extra_input.carbon // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/compile/fail_clang_arg_extra_input.carbon -// CHECK:STDERR: error: unable to handle compilation, expected exactly one compiler job in ' "{{.*}}/clang" "-cc1" {{.*}} "-x" "c++" "bar.cpp"; "{{.*}}/clang" "-cc1" {{.*}} "-x" "c++" ""; ' [CppInteropDriverError] +// CHECK:STDERR: error: unable to handle compilation, expected exactly one compiler job in ' "{{.*}}/clang" "-cc1" {{.*}} "-x" "c++" ""; "{{.*}}/clang" "-cc1" {{.*}} "-x" "c++" "bar.cpp"; ' [CppInteropDriverError] // CHECK:STDERR: // --- foo.carbon diff --git a/toolchain/language_server/context.cpp b/toolchain/language_server/context.cpp index 5600afb569fd..635c8beb8fba 100644 --- a/toolchain/language_server/context.cpp +++ b/toolchain/language_server/context.cpp @@ -169,7 +169,8 @@ auto Context::File::SetText(Context& context, std::optional version, Parse::GetTreeAndSubtreesStore::MakeWithExplicitSize(IdTag(), 1, getter); auto clang_invocation = - BuildClangInvocation(consumer, fs, {context.installation().clang_path()}); + BuildClangInvocation(consumer, fs, context.installation(), + llvm::sys::getDefaultTargetTriple()); Check::CheckParseTrees(units, getters, fs, check_options, std::move(clang_invocation));