diff --git a/bazel/manifest/defs.bzl b/bazel/manifest/defs.bzl index 04695d628742..d3981a76cc2f 100644 --- a/bazel/manifest/defs.bzl +++ b/bazel/manifest/defs.bzl @@ -14,8 +14,11 @@ def _get_files(ctx): ]) if ctx.attr.strip_package_dir: + # Files may or may not be prefixed with the bin directory, and then + # may or may not be prefixed with the package directory. Strip both. + bin_dir = ctx.bin_dir.path + "/" package_dir = ctx.label.package + "/" - files_stripped = [f.removeprefix(package_dir) for f in files] + files_stripped = [f.removeprefix(bin_dir).removeprefix(package_dir) for f in files] else: files_stripped = files diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 8b32a4c08e04..bfeefba008bd 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -135,7 +135,9 @@ cc_library( "//toolchain/sem_ir:file", "//toolchain/sem_ir:formatter", "//toolchain/sem_ir:typed_insts", + "@llvm-project//clang:basic", "@llvm-project//clang:frontend", + "@llvm-project//clang:lex", "@llvm-project//clang:sema", "@llvm-project//clang:tooling", "@llvm-project//llvm:Support", diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 07b5a9592dc8..a4ca8722f491 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -389,7 +389,8 @@ static auto MaybeDumpSemIR( auto CheckParseTrees( llvm::MutableArrayRef units, llvm::ArrayRef tree_and_subtrees_getters, - llvm::IntrusiveRefCntPtr fs, llvm::StringRef target, + llvm::IntrusiveRefCntPtr fs, + llvm::StringRef clang_path, llvm::StringRef target, const CheckParseTreesOptions& options) -> void { // UnitAndImports is big due to its SmallVectors, so we default to 0 on the // stack. @@ -445,7 +446,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]; - CheckUnit(unit_info, tree_and_subtrees_getters, fs, target, + CheckUnit(unit_info, tree_and_subtrees_getters, fs, clang_path, target, options.vlog_stream) .Run(); for (auto* incoming_import : unit_info->incoming_imports) { @@ -494,7 +495,7 @@ auto CheckParseTrees( // incomplete imports. for (auto& unit_info : unit_infos) { if (unit_info.imports_remaining > 0) { - CheckUnit(&unit_info, tree_and_subtrees_getters, fs, target, + CheckUnit(&unit_info, tree_and_subtrees_getters, fs, clang_path, target, options.vlog_stream) .Run(); } diff --git a/toolchain/check/check.h b/toolchain/check/check.h index fe918e2877e3..d3d6ceb36804 100644 --- a/toolchain/check/check.h +++ b/toolchain/check/check.h @@ -74,7 +74,8 @@ struct CheckParseTreesOptions { auto CheckParseTrees( llvm::MutableArrayRef units, llvm::ArrayRef tree_and_subtrees_getters, - llvm::IntrusiveRefCntPtr fs, llvm::StringRef target, + llvm::IntrusiveRefCntPtr fs, + llvm::StringRef clang_path, llvm::StringRef target, const CheckParseTreesOptions& options) -> void; } // namespace Carbon::Check diff --git a/toolchain/check/check_unit.cpp b/toolchain/check/check_unit.cpp index 08419d62c01e..7bd11a56b656 100644 --- a/toolchain/check/check_unit.cpp +++ b/toolchain/check/check_unit.cpp @@ -56,7 +56,8 @@ static auto GetImportedIRCount(UnitAndImports* unit_and_imports) -> int { CheckUnit::CheckUnit( UnitAndImports* unit_and_imports, llvm::ArrayRef tree_and_subtrees_getters, - llvm::IntrusiveRefCntPtr fs, llvm::StringRef target, + llvm::IntrusiveRefCntPtr fs, + llvm::StringRef clang_path, llvm::StringRef target, llvm::raw_ostream* vlog_stream) : unit_and_imports_(unit_and_imports), tree_and_subtrees_getter_( @@ -64,6 +65,7 @@ CheckUnit::CheckUnit( [unit_and_imports->unit->sem_ir->check_ir_id().index]), total_ir_count_(tree_and_subtrees_getters.size()), fs_(std::move(fs)), + clang_path_(clang_path), target_(target), emitter_(&unit_and_imports_->err_tracker, tree_and_subtrees_getters, unit_and_imports_->unit->sem_ir), @@ -156,8 +158,7 @@ auto CheckUnit::InitPackageScopeAndImports() -> void { CARBON_CHECK(cpp_ast); CARBON_CHECK(!cpp_ast->get()); *cpp_ast = - ImportCppFiles(context_, unit_and_imports_->unit->sem_ir->filename(), - cpp_import_names, fs_, target_); + ImportCppFiles(context_, cpp_import_names, fs_, clang_path_, target_); } } diff --git a/toolchain/check/check_unit.h b/toolchain/check/check_unit.h index da871e219c3d..6badd19f3343 100644 --- a/toolchain/check/check_unit.h +++ b/toolchain/check/check_unit.h @@ -124,7 +124,8 @@ class CheckUnit { UnitAndImports* unit_and_imports, llvm::ArrayRef tree_and_subtrees_getters, llvm::IntrusiveRefCntPtr fs, - llvm::StringRef target, llvm::raw_ostream* vlog_stream); + llvm::StringRef clang_path, llvm::StringRef target, + llvm::raw_ostream* vlog_stream); // Produces and checks the IR for the provided unit. auto Run() -> void; @@ -186,6 +187,7 @@ class CheckUnit { // The number of IRs being checked in total. int total_ir_count_; llvm::IntrusiveRefCntPtr fs_; + llvm::StringRef clang_path_; llvm::StringRef target_; DiagnosticEmitter emitter_; diff --git a/toolchain/check/import_cpp.cpp b/toolchain/check/import_cpp.cpp index 7b850fe89ba2..562d2ca02469 100644 --- a/toolchain/check/import_cpp.cpp +++ b/toolchain/check/import_cpp.cpp @@ -10,9 +10,13 @@ #include #include +#include "clang/Basic/FileManager.h" +#include "clang/Frontend/ASTUnit.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/TextDiagnostic.h" +#include "clang/Lex/PreprocessorOptions.h" #include "clang/Sema/Lookup.h" -#include "clang/Tooling/Tooling.h" #include "common/ostream.h" #include "common/raw_string_ostream.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" @@ -30,6 +34,7 @@ #include "toolchain/check/pattern_match.h" #include "toolchain/check/type.h" #include "toolchain/diagnostics/diagnostic.h" +#include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/diagnostics/format_providers.h" #include "toolchain/parse/node_ids.h" #include "toolchain/sem_ir/ids.h" @@ -38,6 +43,9 @@ namespace Carbon::Check { +// The fake file name to use for the synthesized includes file. +static constexpr const char IncludesFileName[] = ""; + // Generates C++ file contents to #include all requested imports. static auto GenerateCppIncludesHeaderCode( Context& context, llvm::ArrayRef imports) @@ -91,13 +99,107 @@ static auto AddImportIRInst(Context& context, namespace { +// Used to convert diagnostics from the Clang driver to Carbon diagnostics. +class CarbonClangDriverDiagnosticConsumer : public clang::DiagnosticConsumer { + public: + // Creates an instance with the location that triggers calling Clang. + // `context` must not be null. + explicit CarbonClangDriverDiagnosticConsumer( + Diagnostics::NoLocEmitter* emitter) + : emitter_(emitter) {} + + // Generates a Carbon warning for each Clang warning and a Carbon error for + // each Clang error or fatal. + auto HandleDiagnostic(clang::DiagnosticsEngine::Level diag_level, + const clang::Diagnostic& info) -> void override { + DiagnosticConsumer::HandleDiagnostic(diag_level, info); + + llvm::SmallString<256> message; + info.FormatDiagnostic(message); + + switch (diag_level) { + case clang::DiagnosticsEngine::Ignored: + case clang::DiagnosticsEngine::Note: + case clang::DiagnosticsEngine::Remark: { + // TODO: Emit notes and remarks. + break; + } + case clang::DiagnosticsEngine::Warning: + case clang::DiagnosticsEngine::Error: + case clang::DiagnosticsEngine::Fatal: { + CARBON_DIAGNOSTIC(CppInteropDriverWarning, Warning, "{0}", std::string); + CARBON_DIAGNOSTIC(CppInteropDriverError, Error, "{0}", std::string); + emitter_->Emit(diag_level == clang::DiagnosticsEngine::Warning + ? CppInteropDriverWarning + : CppInteropDriverError, + message.str().str()); + break; + } + } + } + + private: + // Diagnostic emitter. Note that driver diagnostics don't have meaningful + // locations attached. + Diagnostics::NoLocEmitter* emitter_; +}; + +} // namespace + +// Builds a clang `CompilerInvocation` describing the options to use to build an +// imported C++ AST. +// TODO: Cache the compiler invocation created here and reuse it if building +// multiple AST units. Consider building the `CompilerInvocation` from the +// driver and passing it into check. This would also allow us to have a shared +// set of defaults between the Clang invocation we use for imports and the +// invocation we use for `carbon clang`. +static auto BuildCompilerInvocation( + Context& context, llvm::IntrusiveRefCntPtr fs, + const std::string& clang_path, const std::string& target) + -> std::unique_ptr { + Diagnostics::NoLocEmitter emitter(context.emitter()); + CarbonClangDriverDiagnosticConsumer diagnostics_consumer(&emitter); + + const char* driver_args[] = { + clang_path.c_str(), + // Propagate the target to Clang. + "-target", + target.c_str(), + // Require PIE. Note its default is configurable in Clang. + "-fPIE", + // Parse as a C++ (not C) header. + "-x", + "c++", + IncludesFileName, + }; + + // Build a diagnostics engine. Note that we don't have any diagnostic options + // yet; they're produced by running the driver. + clang::DiagnosticOptions driver_diag_opts; + llvm::IntrusiveRefCntPtr driver_diags( + clang::CompilerInstance::createDiagnostics(*fs, driver_diag_opts, + &diagnostics_consumer, + /*ShouldOwnClient=*/false)); + + // Ask the driver to process the arguments and build a corresponding clang + // frontend invocation. + auto invocation = + clang::createInvocation(driver_args, {.Diags = driver_diags, .VFS = fs}); + + // Emit any queued diagnostics from parsing our driver arguments. + return invocation; +} + +namespace { + // Used to convert Clang diagnostics to Carbon diagnostics. class CarbonClangDiagnosticConsumer : public clang::DiagnosticConsumer { public: // Creates an instance with the location that triggers calling Clang. // `context` must not be null. - explicit CarbonClangDiagnosticConsumer(Context* context) - : context_(context) {} + explicit CarbonClangDiagnosticConsumer(Context* context, + clang::CompilerInvocation* invocation) + : context_(context), invocation_(invocation) {} // Generates a Carbon warning for each Clang warning and a Carbon error for // each Clang error or fatal. @@ -111,16 +213,21 @@ class CarbonClangDiagnosticConsumer : public clang::DiagnosticConsumer { llvm::SmallString<256> message; info.FormatDiagnostic(message); + if (!info.hasSourceManager()) { + // If we don't have a source manager, we haven't actually started + // compiling yet, and this is an error from the driver or early in the + // frontend. Pass it on directly. + CARBON_CHECK(info.getLocation().isInvalid()); + diagnostic_infos_.push_back({.level = diag_level, + .import_ir_inst_id = clang_import_ir_inst_id, + .message = message.str().str()}); + return; + } + RawStringOstream diagnostics_stream; - // TODO: Consider allowing setting `LangOptions` or use - // `ASTContext::getLangOptions()`. - clang::LangOptions lang_options; - // TODO: Consider allowing setting `DiagnosticOptions` or use - // `ASTUnit::getDiagnostics().getLangOptions().getDiagnosticOptions()`. - clang::DiagnosticOptions diagnostic_options; - diagnostic_options.ShowPresumedLoc = true; - clang::TextDiagnostic text_diagnostic(diagnostics_stream, lang_options, - diagnostic_options); + clang::TextDiagnostic text_diagnostic(diagnostics_stream, + invocation_->getLangOpts(), + invocation_->getDiagnosticOpts()); text_diagnostic.emitDiagnostic( clang::FullSourceLoc(info.getLocation(), info.getSourceManager()), diag_level, message, info.getRanges(), info.getFixItHints()); @@ -169,6 +276,9 @@ class CarbonClangDiagnosticConsumer : public clang::DiagnosticConsumer { // The type-checking context in which we're running Clang. Context* context_; + // The compiler invocation that is producing the diagnostics. + clang::CompilerInvocation* invocation_; + // Information on a Clang diagnostic that can be converted to a Carbon // diagnostic. struct ClangDiagnosticInfo { @@ -195,35 +305,51 @@ class CarbonClangDiagnosticConsumer : public clang::DiagnosticConsumer { // compilation errors where encountered or the generated AST is null due to an // error. Sets the AST in the context's `sem_ir`. // TODO: Consider to always have a (non-null) AST. -static auto GenerateAst(Context& context, llvm::StringRef importing_file_path, +static auto GenerateAst(Context& context, llvm::ArrayRef imports, llvm::IntrusiveRefCntPtr fs, - llvm::StringRef target) + const std::string& clang_path, + const std::string& target) -> std::pair, bool> { - CarbonClangDiagnosticConsumer diagnostics_consumer(&context); + // Build the options to use to invoke the Clang frontend. + std::shared_ptr invocation = + BuildCompilerInvocation(context, fs, clang_path, target); + if (!invocation) { + return {nullptr, true}; + } - // TODO: Share compilation flags with ClangRunner. - auto ast = clang::tooling::buildASTFromCodeWithArgs( - GenerateCppIncludesHeaderCode(context, imports), - // Parse C++ (and not C). - { - "-x", - "c++", - // Propagate the target to Clang. - "-target", - target.str(), - // Require PIE. Note its default is configurable in Clang. - "-fPIE", - }, - (importing_file_path + ".generated.cpp_imports.h").str(), "clang-tool", - std::make_shared(), - clang::tooling::getClangStripDependencyFileAdjuster(), - clang::tooling::FileContentMappings(), &diagnostics_consumer, fs); - // Remove link to the diagnostics consumer before its deletion. + // Build a diagnostics engine. + CarbonClangDiagnosticConsumer diagnostics_consumer(&context, + invocation.get()); + llvm::IntrusiveRefCntPtr diags( + clang::CompilerInstance::createDiagnostics( + *fs, invocation->getDiagnosticOpts(), &diagnostics_consumer, + /*ShouldOwnClient=*/false)); + + // Remap the imports file name to the corresponding `#include`s. + std::string includes = GenerateCppIncludesHeaderCode(context, imports); + auto includes_buffer = + llvm::MemoryBuffer::getMemBuffer(includes, IncludesFileName); + invocation->getPreprocessorOpts().addRemappedFile(IncludesFileName, + includes_buffer.get()); + + // Create the AST unit. + auto ast = clang::ASTUnit::LoadFromCompilerInvocation( + invocation, std::make_shared(), nullptr, + diags, new clang::FileManager(invocation->getFileSystemOpts(), fs)); + + // Remove link to the diagnostics consumer before its destruction. ast->getDiagnostics().setClient(nullptr); - // In order to emit diagnostics, we need the AST. + // Remove remapped file before its underlying storage is destroyed. + invocation->getPreprocessorOpts().clearRemappedFiles(); + + // Attach the AST to SemIR. This needs to be done before we can emit any + // diagnostics, so their locations can be properly interpreted by our + // diagnostics machinery. context.sem_ir().set_cpp_ast(ast.get()); + + // Emit any diagnostics we queued up while building the AST. diagnostics_consumer.EmitDiagnostics(); return {std::move(ast), !ast || diagnostics_consumer.getNumErrors() > 0}; @@ -257,10 +383,11 @@ static auto AddNamespace(Context& context, PackageNameId cpp_package_id, .add_result.name_scope_id; } -auto ImportCppFiles(Context& context, llvm::StringRef importing_file_path, +auto ImportCppFiles(Context& context, llvm::ArrayRef imports, llvm::IntrusiveRefCntPtr fs, - llvm::StringRef target) -> std::unique_ptr { + llvm::StringRef clang_path, llvm::StringRef target) + -> std::unique_ptr { if (imports.empty()) { return nullptr; } @@ -275,7 +402,7 @@ auto ImportCppFiles(Context& context, llvm::StringRef importing_file_path, auto name_scope_id = AddNamespace(context, package_id, imports); auto [generated_ast, ast_has_error] = - GenerateAst(context, importing_file_path, imports, fs, target); + GenerateAst(context, imports, fs, clang_path.str(), target.str()); SemIR::NameScope& name_scope = context.name_scopes().Get(name_scope_id); name_scope.set_is_closed_import(true); diff --git a/toolchain/check/import_cpp.h b/toolchain/check/import_cpp.h index df950be88442..141a2485b829 100644 --- a/toolchain/check/import_cpp.h +++ b/toolchain/check/import_cpp.h @@ -14,10 +14,11 @@ namespace Carbon::Check { // Generates a C++ header that includes the imported cpp files, parses it, // generates the AST from it and links `SemIR::File` to it. Report C++ errors // and warnings. If successful, adds a `Cpp` namespace and returns the AST. -auto ImportCppFiles(Context& context, llvm::StringRef importing_file_path, +auto ImportCppFiles(Context& context, llvm::ArrayRef imports, llvm::IntrusiveRefCntPtr fs, - llvm::StringRef target) -> std::unique_ptr; + llvm::StringRef clang_path, llvm::StringRef target) + -> std::unique_ptr; // Looks up the given name in the Clang AST generated when importing C++ code. // If successful, generates the instruction and returns the new `InstId`. diff --git a/toolchain/check/testdata/interop/cpp/include_paths.carbon b/toolchain/check/testdata/interop/cpp/include_paths.carbon new file mode 100644 index 000000000000..66ff5a3cb3ab --- /dev/null +++ b/toolchain/check/testdata/interop/cpp/include_paths.carbon @@ -0,0 +1,53 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/uint.carbon +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/interop/cpp/include_paths.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interop/cpp/include_paths.carbon + +// --- include_stddef.h + +// Check that include paths are properly set up when running Clang. + +// Clang provides as a builtin header. +#include + +ptrdiff_t GetSize(); +inline int GetSizeAsInt() { return GetSize(); } + +// --- import_stddef_indirectly.carbon + +library "[[@TEST_NAME]]"; + +import Cpp library "include_stddef.h"; + +// TODO: `fn CallGetSize() -> Cpp.ptrdiff_t {` +fn CallGetSize() -> i32 { + // TODO: Call `Cpp.GetSize` directly once we can import the types `long` and `long long`. + return Cpp.GetSizeAsInt() as i32; +} + +// --- fail_todo_import_stddef_directly.carbon + +library "[[@TEST_NAME]]"; + +import Cpp library "stddef.h"; + +// TODO: Once we can import typedefs, this should work. +// CHECK:STDERR: fail_todo_import_stddef_directly.carbon:[[@LINE+11]]:8: error: semantics TODO: `Unsupported: Declaration type Typedef` [SemanticsTodo] +// CHECK:STDERR: var n: Cpp.size_t; +// CHECK:STDERR: ^~~~~~~~~~ +// CHECK:STDERR: fail_todo_import_stddef_directly.carbon:[[@LINE+8]]:8: note: in `Cpp` name lookup for `size_t` [InCppNameLookup] +// CHECK:STDERR: var n: Cpp.size_t; +// CHECK:STDERR: ^~~~~~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_todo_import_stddef_directly.carbon:[[@LINE+4]]:8: error: member name `size_t` not found in `Cpp` [MemberNameNotFoundInInstScope] +// CHECK:STDERR: var n: Cpp.size_t; +// CHECK:STDERR: ^~~~~~~~~~ +// CHECK:STDERR: +var n: Cpp.size_t; diff --git a/toolchain/diagnostics/coverage_test.cpp b/toolchain/diagnostics/coverage_test.cpp index d807c1eb4ea5..915f6395cf97 100644 --- a/toolchain/diagnostics/coverage_test.cpp +++ b/toolchain/diagnostics/coverage_test.cpp @@ -58,6 +58,12 @@ constexpr Kind UntestedKinds[] = { // conversion cannot fail. This should be covered once we support `ref` // binding syntax. Kind::ConversionFailureNonRefToRef, + + // TODO: These are temporarily unreachable because we don't pass invalid + // driver options to Clang, but will become reachable once we support + // passing custom Clang arguments. + Kind::CppInteropDriverError, + Kind::CppInteropDriverWarning, }; // Looks for diagnostic kinds that aren't covered by a file_test. diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h index 80fda494c074..6a8b4e30549c 100644 --- a/toolchain/diagnostics/diagnostic_emitter.h +++ b/toolchain/diagnostics/diagnostic_emitter.h @@ -186,6 +186,7 @@ class Emitter { template friend class AnnotationScope; + friend class NoLocEmitter; Consumer* consumer_; llvm::SmallVectorvoid>> @@ -203,6 +204,10 @@ class NoLocEmitter : public Emitter { public: using Emitter::Emitter; + template + explicit NoLocEmitter(const Emitter& emitter) + : Emitter(emitter.consumer_) {} + // Emits an error. This specialization only applies to // `NoLocEmitter`. template diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index dc7296f9ae50..7fdaadddadf1 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -166,6 +166,8 @@ CARBON_DIAGNOSTIC_KIND(ResolvingSpecificHere) // Package/import checking diagnostics. CARBON_DIAGNOSTIC_KIND(CppInteropFuzzing) CARBON_DIAGNOSTIC_KIND(CppInteropMissingLibrary) +CARBON_DIAGNOSTIC_KIND(CppInteropDriverError) +CARBON_DIAGNOSTIC_KIND(CppInteropDriverWarning) CARBON_DIAGNOSTIC_KIND(CppInteropParseError) CARBON_DIAGNOSTIC_KIND(CppInteropParseWarning) CARBON_DIAGNOSTIC_KIND(IncorrectExtension) diff --git a/toolchain/driver/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp index b145082f731d..33734750b50f 100644 --- a/toolchain/driver/compile_subcommand.cpp +++ b/toolchain/driver/compile_subcommand.cpp @@ -944,8 +944,8 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { } } Check::CheckParseTrees(check_units, cache.tree_and_subtrees_getters(), - driver_env.fs, options_.codegen_options.target, - options); + driver_env.fs, driver_env.installation->clang_path(), + options_.codegen_options.target, options); CARBON_VLOG_TO(driver_env.vlog_stream, "*** Check::CheckParseTrees done ***\n"); for (auto& unit : units) { diff --git a/toolchain/driver/language_server_subcommand.cpp b/toolchain/driver/language_server_subcommand.cpp index 0a0319cbf4cb..478c4855a997 100644 --- a/toolchain/driver/language_server_subcommand.cpp +++ b/toolchain/driver/language_server_subcommand.cpp @@ -27,9 +27,10 @@ auto LanguageServerSubcommand::Run(DriverEnv& driver_env) -> DriverResult { return {.success = false}; } - bool success = LanguageServer::Run( - driver_env.input_stream, *driver_env.output_stream, - *driver_env.error_stream, driver_env.vlog_stream, driver_env.consumer); + bool success = + LanguageServer::Run(*driver_env.installation, driver_env.input_stream, + *driver_env.output_stream, *driver_env.error_stream, + driver_env.vlog_stream, driver_env.consumer); return {.success = success}; } diff --git a/toolchain/install/BUILD b/toolchain/install/BUILD index 8fbfd343ddcb..ef415be49989 100644 --- a/toolchain/install/BUILD +++ b/toolchain/install/BUILD @@ -181,7 +181,12 @@ install_dirs = { is_driver = True, ) for name in llvm_binaries], "lib/carbon/llvm/lib/clang/" + LLVM_VERSION_MAJOR: [ - install_filegroup("include", ":clang_headers", "staging/include/"), + install_filegroup( + "include", + ":clang_headers", + label = "installed_clang_headers", + remove_prefix = "staging/include/", + ), ], } @@ -205,6 +210,14 @@ manifest( srcs = [":install_data"], ) +# A list of clang's installed builtin header files. +# This is consumed by //toolchain/testing:file_test. +manifest( + name = "clang_headers_manifest.txt", + srcs = [":installed_clang_headers"], + strip_package_dir = True, +) + pkg_naming_variables( name = "packaging_variables", ) diff --git a/toolchain/install/install_filegroups.bzl b/toolchain/install/install_filegroups.bzl index 1cafe4f3cdf2..de02df18de10 100644 --- a/toolchain/install/install_filegroups.bzl +++ b/toolchain/install/install_filegroups.bzl @@ -7,7 +7,7 @@ load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_filegroup", "pkg_files", "pkg_mklink", "strip_prefix") load("symlink_helpers.bzl", "symlink_file", "symlink_filegroup") -def install_filegroup(name, filegroup_target, remove_prefix = ""): +def install_filegroup(name, filegroup_target, remove_prefix = "", label = None): """Adds a filegroup for install. Used in the `install_dirs` dict. @@ -15,10 +15,15 @@ def install_filegroup(name, filegroup_target, remove_prefix = ""): Args: name: The base directory for the filegroup. filegroup_target: The bazel filegroup target to install. + remove_prefix: A prefix to remove from the name of each source file when + determining the name of the corresponding installed file. + label: A custom label to assign to the filegroup containing the + installed files. """ return { "filegroup": filegroup_target, "is_driver": False, + "label": label, "name": name, "remove_prefix": remove_prefix, } @@ -85,8 +90,8 @@ def make_install_filegroups(name, no_driver_name, pkg_name, install_dirs, prefix if not entry["is_driver"]: no_driver_srcs.append(prefixed_path) - pkg_path = path + ".pkg" - pkg_srcs.append(pkg_path) + pkg_label = entry.get("label") or path + ".pkg" + pkg_srcs.append(pkg_label) if "target" in entry: if entry["executable"]: @@ -102,7 +107,7 @@ def make_install_filegroups(name, no_driver_name, pkg_name, install_dirs, prefix ) mode = "0644" pkg_files( - name = pkg_path, + name = pkg_label, srcs = [entry["target"]], attributes = pkg_attributes(mode = mode), renames = {entry["target"]: path}, @@ -115,7 +120,7 @@ def make_install_filegroups(name, no_driver_name, pkg_name, install_dirs, prefix remove_prefix = entry["remove_prefix"], ) pkg_files( - name = pkg_path, + name = pkg_label, srcs = [prefixed_path], strip_prefix = strip_prefix.from_pkg(prefix), ) @@ -139,7 +144,7 @@ def make_install_filegroups(name, no_driver_name, pkg_name, install_dirs, prefix # For the distributed package, we retain relative symlinks. pkg_mklink( - name = pkg_path, + name = pkg_label, link_name = path, target = entry["symlink"], ) diff --git a/toolchain/install/install_paths.cpp b/toolchain/install/install_paths.cpp index 0bd18db6e266..653d2d381c97 100644 --- a/toolchain/install/install_paths.cpp +++ b/toolchain/install/install_paths.cpp @@ -92,19 +92,32 @@ auto InstallPaths::Make(llvm::StringRef install_prefix) -> InstallPaths { auto InstallPaths::ReadPreludeManifest() const -> ErrorOr> { + return ReadManifest(core_package(), "prelude_manifest.txt"); +} + +auto InstallPaths::ReadClangHeadersManifest() const + -> ErrorOr> { + llvm::SmallString<256> manifest_path(prefix_); + llvm::sys::path::append(manifest_path, llvm::sys::path::Style::posix, ".."); + return ReadManifest(manifest_path, "clang_headers_manifest.txt"); +} + +auto InstallPaths::ReadManifest(llvm::StringRef manifest_path, + llvm::StringRef manifest_file) const + -> ErrorOr> { // This is structured to avoid a vector copy on success. ErrorOr> result = llvm::SmallVector(); llvm::SmallString<256> manifest; llvm::sys::path::append(manifest, llvm::sys::path::Style::posix, - core_package(), "prelude_manifest.txt"); + manifest_path, manifest_file); auto fs = llvm::vfs::getRealFileSystem(); llvm::ErrorOr> file = fs->getBufferForFile(manifest); if (!file) { - result = ErrorBuilder() << "Loading prelude manifest `" << manifest + result = ErrorBuilder() << "Loading manifest `" << manifest << "`: " << file.getError().message(); return result; } @@ -117,14 +130,14 @@ auto InstallPaths::ReadPreludeManifest() const break; } llvm::SmallString<256> path; - llvm::sys::path::append(path, llvm::sys::path::Style::posix, core_package(), + llvm::sys::path::append(path, llvm::sys::path::Style::posix, manifest_path, token); result->push_back(path.str().str()); buffer = remainder; } if (result->empty()) { - result = ErrorBuilder() << "Prelude manifest `" << manifest << "` is empty"; + result = ErrorBuilder() << "Manifest `" << manifest << "` is empty"; } return result; } diff --git a/toolchain/install/install_paths.h b/toolchain/install/install_paths.h index fa3bc9e24f3c..96dca2f1609b 100644 --- a/toolchain/install/install_paths.h +++ b/toolchain/install/install_paths.h @@ -68,6 +68,12 @@ class InstallPaths { // files that define the prelude, and will always be non-empty on success. auto ReadPreludeManifest() const -> ErrorOr>; + // Returns the contents of the clang builtin headers manifest file. This is + // the list of header files that are installed as part of the clang compiler, + // and will always be non-empty on success. + auto ReadClangHeadersManifest() const + -> ErrorOr>; + // Check for an error detecting the install paths correctly. // // A nullopt return means no errors encountered and the paths should work @@ -111,6 +117,11 @@ class InstallPaths { // relevant error message. auto CheckMarkerFile() -> void; + // Read a manifest file. + auto ReadManifest(llvm::StringRef manifest_path, + llvm::StringRef manifest_file) const + -> ErrorOr>; + // The computed installation prefix. This will be an absolute path. We keep an // absolute path for when the command line uses a relative path // (`./bin/carbon`) and the working directory changes after initialization diff --git a/toolchain/language_server/BUILD b/toolchain/language_server/BUILD index 1b62ceb68193..a029d1e2dd1c 100644 --- a/toolchain/language_server/BUILD +++ b/toolchain/language_server/BUILD @@ -22,6 +22,7 @@ cc_library( "//common:ostream", "//common:raw_string_ostream", "//toolchain/diagnostics:diagnostic_emitter", + "//toolchain/install:install_paths", "@llvm-project//clang-tools-extra/clangd:ClangDaemon", ], ) @@ -38,6 +39,7 @@ cc_library( "//toolchain/check", "//toolchain/diagnostics:diagnostic_emitter", "//toolchain/diagnostics:file_diagnostics", + "//toolchain/install:install_paths", "//toolchain/lex", "//toolchain/lex:tokenized_buffer", "//toolchain/parse", diff --git a/toolchain/language_server/context.cpp b/toolchain/language_server/context.cpp index b2fc98bed59e..26b74896d57d 100644 --- a/toolchain/language_server/context.cpp +++ b/toolchain/language_server/context.cpp @@ -163,9 +163,10 @@ auto Context::File::SetText(Context& context, std::optional version, // TODO: Include the prelude. Check::CheckParseTreesOptions check_options; check_options.vlog_stream = context.vlog_stream(); - Check::CheckParseTrees( - units, llvm::ArrayRef(getter), fs, - llvm::sys::getDefaultTargetTriple(), check_options); + Check::CheckParseTrees(units, + llvm::ArrayRef(getter), + fs, context.installation().clang_path(), + llvm::sys::getDefaultTargetTriple(), check_options); // Note we need to publish diagnostics even when empty. // TODO: Consider caching previously published diagnostics and only publishing diff --git a/toolchain/language_server/context.h b/toolchain/language_server/context.h index 231911ec34f1..2804397c6793 100644 --- a/toolchain/language_server/context.h +++ b/toolchain/language_server/context.h @@ -14,6 +14,7 @@ #include "toolchain/diagnostics/diagnostic_consumer.h" #include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/diagnostics/file_diagnostics.h" +#include "toolchain/install/install_paths.h" #include "toolchain/lex/tokenized_buffer.h" #include "toolchain/parse/tree_and_subtrees.h" #include "toolchain/sem_ir/file.h" @@ -52,10 +53,12 @@ class Context { }; // `vlog_stream` is optional; other parameters are required. - explicit Context(llvm::raw_ostream* vlog_stream, + explicit Context(const InstallPaths* installation, + llvm::raw_ostream* vlog_stream, Diagnostics::Consumer* consumer, clang::clangd::LSPBinder::RawOutgoing* outgoing) - : vlog_stream_(vlog_stream), + : installation_(installation), + vlog_stream_(vlog_stream), file_emitter_(consumer), no_loc_emitter_(consumer), outgoing_(outgoing) {} @@ -70,6 +73,8 @@ class Context { outgoing_->notify("textDocument/publishDiagnostics", params); } + auto installation() -> const InstallPaths& { return *installation_; } + auto vlog_stream() -> llvm::raw_ostream* { return vlog_stream_; } auto file_emitter() -> Diagnostics::FileEmitter& { return file_emitter_; } auto no_loc_emitter() -> Diagnostics::NoLocEmitter& { @@ -79,6 +84,8 @@ class Context { auto files() -> Map& { return files_; } private: + const InstallPaths* installation_; + // Diagnostic and output streams. llvm::raw_ostream* vlog_stream_; Diagnostics::FileEmitter file_emitter_; diff --git a/toolchain/language_server/language_server.cpp b/toolchain/language_server/language_server.cpp index b40d11621745..74ae2a26bccb 100644 --- a/toolchain/language_server/language_server.cpp +++ b/toolchain/language_server/language_server.cpp @@ -44,9 +44,10 @@ class Logger : public clang::clangd::Logger { std::unique_ptr vlog_logger_; }; -auto Run(FILE* input_stream, llvm::raw_ostream& output_stream, - llvm::raw_ostream& error_stream, llvm::raw_ostream* vlog_stream, - Diagnostics::Consumer& consumer) -> bool { +auto Run(const InstallPaths& installation, FILE* input_stream, + llvm::raw_ostream& output_stream, llvm::raw_ostream& error_stream, + llvm::raw_ostream* vlog_stream, Diagnostics::Consumer& consumer) + -> bool { // The language server internally uses diagnostics for logging issues, but the // clangd parts have their own logging system. We intercept that here. Logger logger(&error_stream, vlog_stream); @@ -58,7 +59,7 @@ auto Run(FILE* input_stream, llvm::raw_ostream& output_stream, /*InMirror=*/nullptr, /*Pretty=*/true)); OutgoingMessages outgoing(transport.get()); - Context context(vlog_stream, &consumer, &outgoing); + Context context(&installation, vlog_stream, &consumer, &outgoing); IncomingMessages incoming(transport.get(), &context); // Run the server loop. diff --git a/toolchain/language_server/language_server.h b/toolchain/language_server/language_server.h index c5d148c533ca..71e21726e300 100644 --- a/toolchain/language_server/language_server.h +++ b/toolchain/language_server/language_server.h @@ -7,6 +7,7 @@ #include "common/ostream.h" #include "toolchain/diagnostics/diagnostic_consumer.h" +#include "toolchain/install/install_paths.h" namespace Carbon::LanguageServer { @@ -15,9 +16,10 @@ namespace Carbon::LanguageServer { // the server cleanly exits. // // This is thread-hostile because `clangd::LoggingSession` relies on a global. -auto Run(FILE* input_stream, llvm::raw_ostream& output_stream, - llvm::raw_ostream& error_stream, llvm::raw_ostream* vlog_stream, - Diagnostics::Consumer& consumer) -> bool; +auto Run(const InstallPaths& installation, FILE* input_stream, + llvm::raw_ostream& output_stream, llvm::raw_ostream& error_stream, + llvm::raw_ostream* vlog_stream, Diagnostics::Consumer& consumer) + -> bool; } // namespace Carbon::LanguageServer diff --git a/toolchain/sem_ir/diagnostic_loc_converter.cpp b/toolchain/sem_ir/diagnostic_loc_converter.cpp index 86aebd5c30da..a1eb6eab99e7 100644 --- a/toolchain/sem_ir/diagnostic_loc_converter.cpp +++ b/toolchain/sem_ir/diagnostic_loc_converter.cpp @@ -65,6 +65,9 @@ auto DiagnosticLocConverter::ConvertImpl( CARBON_CHECK(sem_ir_->cpp_ast()); clang::PresumedLoc presumed_loc = sem_ir_->cpp_ast()->getSourceManager().getPresumedLoc(clang_loc); + if (presumed_loc.isInvalid()) { + return Diagnostics::ConvertedLoc(); + } unsigned offset = sem_ir_->cpp_ast()->getSourceManager().getDecomposedLoc(clang_loc).second; diff --git a/toolchain/testing/BUILD b/toolchain/testing/BUILD index 39e988e795e1..6e6faa1388d9 100644 --- a/toolchain/testing/BUILD +++ b/toolchain/testing/BUILD @@ -60,7 +60,11 @@ file_test( size = "small", timeout = "moderate", # Taking >60 seconds in GitHub actions srcs = ["file_test.cpp"], - data = [":min_prelude"], + data = [ + ":min_prelude", + "//toolchain/install:clang_headers", + "//toolchain/install:clang_headers_manifest.txt", + ], tests = [":all_testdata"], deps = [ "//common:all_llvm_targets", diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index fa542a2bd7b3..4aefaef92fda 100644 --- a/toolchain/testing/file_test.cpp +++ b/toolchain/testing/file_test.cpp @@ -24,6 +24,56 @@ namespace Carbon::Testing { namespace { +// Adds a file to the fs. +static auto AddFile(llvm::vfs::InMemoryFileSystem& fs, llvm::StringRef path) + -> ErrorOr { + llvm::ErrorOr> file = + llvm::MemoryBuffer::getFile(path); + if (file.getError()) { + return ErrorBuilder() << "Getting `" << path + << "`: " << file.getError().message(); + } + if (!fs.addFile(path, /*ModificationTime=*/0, std::move(*file))) { + return ErrorBuilder() << "Duplicate file: `" << path << "`"; + } + return Success(); +} + +struct SharedTestData { + // The toolchain install information. + InstallPaths installation; + + // Files in the prelude. + llvm::SmallVector prelude_files; + + // The installed files that tests can use. + llvm::IntrusiveRefCntPtr file_system = + new llvm::vfs::InMemoryFileSystem(); +}; + +static auto GetSharedTestData(llvm::StringRef exe_path) + -> const SharedTestData* { + static ErrorOr data = [&]() -> ErrorOr { + SharedTestData data = {.installation = + InstallPaths::MakeForBazelRunfiles(exe_path)}; + CARBON_ASSIGN_OR_RETURN(data.prelude_files, + data.installation.ReadPreludeManifest()); + for (const auto& file : data.prelude_files) { + CARBON_RETURN_IF_ERROR(AddFile(*data.file_system, file)); + } + + llvm::SmallVector clang_header_files; + CARBON_ASSIGN_OR_RETURN(clang_header_files, + data.installation.ReadClangHeadersManifest()); + for (const auto& file : clang_header_files) { + CARBON_RETURN_IF_ERROR(AddFile(*data.file_system, file)); + } + return data; + }(); + CARBON_CHECK(data.ok(), "{0}", data.error()); + return &*data; +} + // Provides common test support for the driver. This is used by file tests in // component subdirectories. class ToolchainFileTest : public FileTestBase { @@ -62,8 +112,8 @@ class ToolchainFileTest : public FileTestBase { private: // The toolchain component subdirectory, such as `lex` or `language_server`. const llvm::StringRef component_; - // The toolchain install information. - const InstallPaths installation_; + // The shared test data. + const SharedTestData* data_; }; } // namespace @@ -85,37 +135,16 @@ ToolchainFileTest::ToolchainFileTest(llvm::StringRef exe_path, llvm::StringRef test_name) : FileTestBase(test_name), component_(GetComponent(test_name)), - installation_(InstallPaths::MakeForBazelRunfiles(exe_path)) {} - -// Adds a file to the fs. -static auto AddFile(llvm::vfs::InMemoryFileSystem& fs, llvm::StringRef path) - -> ErrorOr { - llvm::ErrorOr> file = - llvm::MemoryBuffer::getFile(path); - if (file.getError()) { - return ErrorBuilder() << "Getting `" << path - << "`: " << file.getError().message(); - } - if (!fs.addFile(path, /*ModificationTime=*/0, std::move(*file))) { - return ErrorBuilder() << "Duplicate file: `" << path << "`"; - } - return Success(); -} + data_(GetSharedTestData(exe_path)) {} auto ToolchainFileTest::Run( const llvm::SmallVector& test_args, llvm::IntrusiveRefCntPtr& fs, FILE* input_stream, llvm::raw_pwrite_stream& output_stream, llvm::raw_pwrite_stream& error_stream) const -> ErrorOr { - llvm::SmallVector prelude_files; - // Lex and parse shouldn't ever access the prelude. - if (component_ != "lex" && component_ != "parse") { - // TODO: Try providing the prelude as an overlay. - CARBON_ASSIGN_OR_RETURN(prelude_files, installation_.ReadPreludeManifest()); - for (const auto& file : prelude_files) { - CARBON_RETURN_IF_ERROR(AddFile(*fs, file)); - } - } + llvm::IntrusiveRefCntPtr overlay_fs = + new llvm::vfs::OverlayFileSystem(data_->file_system); + overlay_fs->pushOverlay(fs); llvm::SmallVector filtered_test_args; if (component_ == "check" || component_ == "lower") { @@ -144,7 +173,7 @@ auto ToolchainFileTest::Run( filtered_test_args = test_args; } - Driver driver(fs, &installation_, input_stream, &output_stream, + Driver driver(overlay_fs, &data_->installation, input_stream, &output_stream, &error_stream); auto driver_result = driver.RunCommand(filtered_test_args); // If any diagnostics have been produced, add a trailing newline to make the @@ -164,7 +193,7 @@ auto ToolchainFileTest::Run( [&](std::pair entry) { return entry.first == "." || entry.first == "-" || entry.first.starts_with("not_file") || - llvm::is_contained(prelude_files, entry.first); + llvm::is_contained(data_->prelude_files, entry.first); }); if (component_ == "language_server") { @@ -187,13 +216,13 @@ auto ToolchainFileTest::GetDefaultArgs() const return args; } - args.insert(args.end(), - { - "compile", - "--phase=" + component_.str(), - // Use the install path to exclude prelude files. - "--exclude-dump-file-prefix=" + installation_.core_package(), - }); + args.insert(args.end(), { + "compile", + "--phase=" + component_.str(), + // Use the install path to exclude prelude files. + "--exclude-dump-file-prefix=" + + data_->installation.core_package(), + }); if (component_ == "lex") { args.insert(args.end(), {"--no-prelude-import", "--dump-tokens", @@ -257,7 +286,7 @@ auto ToolchainFileTest::DoExtraCheckReplacements(std::string& check_line) const // TODO: Consider adding a content keyword to name the core package, and // replace with that instead. Alternatively, consider adding the core // package to the VFS with a fixed name. - absl::StrReplaceAll({{installation_.core_package(), "{{.*}}"}}, + absl::StrReplaceAll({{data_->installation.core_package(), "{{.*}}"}}, &check_line); } else { FileTestBase::DoExtraCheckReplacements(check_line);