d076608d58d1ec55016eb747a995511e3a3f72aa moved some deps around to avoid cycles and left clang/Frontend/FrontendDiagnostic.h as a shim that simply includes clang/Basic/DiagnosticFrontend.h. This PR inlines it so that nothing in tree still includes clang/Frontend/FrontendDiagnostic.h. Doing this will help prevent future layering issues. See #162865. Frontend already depends on Basic, so no new deps need to be added anywhere except for places that do strict dep checking.
This aligns `ClangDiagnosticManagerAdapter` with how we set up the diagnostics in `ClangModulesDeclVendor`. We fixed lifetime issues around the same kind of setup here: https://github.com/llvm/llvm-project/pull/167724 This class didn't suffer from the same lifetime issue because it used `shared_ptr`s. So the stream wasn't freed before `~TextDiagnosticPrinter` accessing it. But that begged the question of why these are `shared_ptr`s in the first place. This patch makes these `unique_ptr`s and fixes the destruction order that would now be an issue.
Instead of propagating the errors as a `bool`+`Stream` we change the `ClangModulesDeclVendor` module loading APIs to use `llvm::Error`. We also reword some of the diagnostics (notably removing the hardcoded `error:` prefix). A follow-up patch will further make the module loading errors less noisy. See the new tests for what the errors look like. rdar://164002569
This avoids unintentional comparisons between `SourceLanguage` and `LanguageType`. Also marks `operator bool` explicit so we don't implicitly convert to bool.
Since https://github.com/llvm/llvm-project/pull/158381 the `CompilerInstance` is aware of the VFS and co-owns it. To reduce scope of that PR, the VFS was being inherited from the `FileManager` during `setFileManager()` if it wasn't configured before. However, the implementation of that setter was buggy. This PR fixes the bug, and moves us closer to the long-term goal of `CompilerInstance` requiring the VFS to be configured explicitly and owned by the instance.
Depends on: * https://github.com/llvm/llvm-project/pull/162050 Since it's a 'Note' diagnostic it would only show up when expression evaluation actually failed. This helps with expression evaluation failure reports in mixed language environments where it's not quite clear what language the expression ran as. It may also reduce confusion around why the expression evaluator ran an expression in a language it wasn't asked to run (a softer alternative to what I attempted in https://github.com/llvm/llvm-project/pull/156648). Here are some example outputs: ``` # Without target (lldb) expr blah note: Falling back to default language. Ran expression as 'Objective C++'. # Stopped in target (lldb) expr blah note: Ran expression as 'C++14'. (lldb) expr -l objc -- blah note: Expression evaluation in pure Objective-C not supported. Ran expression as 'Objective C++'. (lldb) expr -l c -- blah note: Expression evaluation in pure C not supported. Ran expression as 'ISO C++'. (lldb) expr -l c++14 -- blah note: Ran expression as 'C++14' (lldb) expr -l c++20 -- blah note: Ran expression as 'C++20' (lldb) expr -l objective-c++ -- blah note: Ran expression as 'Objective C++' (lldb) expr -l D -- blah note: Expression evaluation in D not supported. Falling back to default language. Ran expression as 'Objective C++'. ``` I didn't put the diagnostic on the same line as the inline diagnostic for now because of implementation convenience, but if reviewers deem that a blocker I can take a stab at that again. Also, other language plugins (namely Swift), won't immediately benefit from this and will have to emit their own diagnistc. I played around with having a virtual API on `UserExpression` or `ExpressionParser` that will be called consistently, but by the time we're about to parse the expression we are already several frames deep into the plugin. Before (and at the beginning of) the generic `UserExpression::Parse` call we don't have enough information to notify which language we're going to parse in (at least for the C++ plugin). rdar://160297649 rdar://159669244
The `CompilerInstance::createSourceManager()` function currently accepts the `FileManager` to be used. However, all clients call `CompilerInstance::createFileManager()` prior to creating the `SourceManager`, and it never makes sense to use a `FileManager` in the `SourceManager` that's different from the rest of the compiler. Passing the `FileManager` explicitly is redundant, error-prone, and deviates from the style of other `CompilerInstance` initialization APIs. This PR therefore removes the `FileManager` parameter from `createSourceManager()` and also stops returning the `FileManager` pointer from `createFileManager()`, since that was its primary use. Now, `createSourceManager()` internally calls `getFileManager()` instead.
Here's an example crash that we've seen sporadically over the years: ``` 0 libsystem_kernel.dylib 0x19d392388 __pthread_kill + 8 1 libsystem_pthread.dylib 0x19d3cb88c pthread_kill + 296 2 libsystem_c.dylib 0x19d29cd04 raise + 32 3 LLDB 0x112cbbc94 SignalHandler(int, __siginfo*, void*) + 324 4 libsystem_platform.dylib 0x19d4056a4 _sigtramp + 56 5 LLDB 0x110dcbd38 clang::TextDiagnosticPrinter::HandleDiagnostic(clang::DiagnosticsEngine::Level, clang::Diagnostic const&) + 1216 6 LLDB 0x110dcbd38 clang::TextDiagnosticPrinter::HandleDiagnostic(clang::DiagnosticsEngine::Level, clang::Diagnostic const&) + 1216 7 LLDB 0x10de12128 ClangDiagnosticManagerAdapter::HandleDiagnostic(clang::DiagnosticsEngine::Level, clang::Diagnostic const&) + 332 8 LLDB 0x1121eb3dc clang::DiagnosticIDs::ProcessDiag(clang::DiagnosticsEngine&) const + 200 9 LLDB 0x1121e67a0 clang::DiagnosticsEngine::EmitCurrentDiagnostic(bool) + 100 10 LLDB 0x111d766cc IsStructurallyEquivalent(clang::StructuralEquivalenceContext&, clang::FieldDecl*, clang::FieldDecl*, clang::QualType) + 1568 11 LLDB 0x111d71b54 IsStructurallyEquivalent(clang::StructuralEquivalenceContext&, clang::RecordDecl*, clang::RecordDecl*) + 2076 12 LLDB 0x111d6e448 clang::StructuralEquivalenceContext::Finish() + 204 13 LLDB 0x111d6e1e0 clang::StructuralEquivalenceContext::IsEquivalent(clang::Decl*, clang::Decl*) + 32 14 LLDB 0x111d3b788 clang::ASTNodeImporter::IsStructuralMatch(clang::Decl*, clang::Decl*, bool, bool) + 168 15 LLDB 0x111d404e0 clang::ASTNodeImporter::VisitFunctionDecl(clang::FunctionDecl*) + 1300 16 LLDB 0x111d5cae0 clang::ASTImporter::ImportImpl(clang::Decl*) + 24 17 LLDB 0x10ddf30bc lldb_private::ClangASTImporter::ASTImporterDelegate::ImportImpl(clang::Decl*) + 308 18 LLDB 0x111d48140 clang::ASTImporter::Import(clang::Decl*) + 984 19 LLDB 0x10ddee9dc lldb_private::ClangASTImporter::CopyDecl(clang::ASTContext*, clang::Decl*) + 100 20 LLDB 0x10ddfab40 lldb_private::ClangASTSource::FindExternalLexicalDecls(clang::DeclContext const*, llvm::function_ref<bool (clang::Decl::Kind)>, llvm::SmallVectorImpl<clang::Decl*>&) + 1692 21 LLDB 0x111e1cb84 clang::DeclContext::LoadLexicalDeclsFromExternalStorage() const + 180 22 LLDB 0x111e1df50 clang::DeclContext::buildLookup() + 204 23 LLDB 0x111e1dcf4 clang::DeclContext::makeDeclVisibleInContextWithFlags(clang::NamedDecl*, bool, bool) + 504 24 LLDB 0x111d3b01c clang::ASTNodeImporter::ImportDeclContext(clang::DeclContext*, bool) + 724 25 LLDB 0x111d62d10 clang::ASTImporter::ImportDefinition(clang::Decl*) + 428 26 LLDB 0x10ddf1cb0 lldb_private::ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(clang::Decl*, clang::Decl*) + 524 27 LLDB 0x10ddef3c8 (anonymous namespace)::CompleteTagDeclsScope::~CompleteTagDeclsScope() + 616 28 LLDB 0x10ddef6dc lldb_private::ClangASTImporter::DeportDecl(clang::ASTContext*, clang::Decl*) + 436 29 LLDB 0x10ddec3dc lldb_private::ASTResultSynthesizer::CommitPersistentDecls() + 236 30 LLDB 0x10de1091c lldb_private::ClangExpressionParser::ParseInternal(lldb_private::DiagnosticManager&, clang::CodeCompleteConsumer*, unsigned int, unsigned int) + 2292 31 LLDB 0x10de21238 lldb_private::ClangUserExpression::TryParse(lldb_private::DiagnosticManager&, lldb_private::ExecutionContext&, lldb_private::ExecutionPolicy, bool, bool) + 328 ... ``` Here `ASTImporter::IsStructurallyEquivalent` is trying to emit a diagnostic using `ClangExpressionParser`'s `ClangDiagnosticManagerAdapter`. But `TextDiagnosticPrinter::TextDiag` seems to be `nullptr`. This can only happen when we call `HandleDiagnostic` on `ClangDiagnosticManagerAdapter` after we called `EndSourceFile`. Specifically, it looks like when moving a type from `Expression` AST to `Scratch` AST (in `CommitPersistentDecls`), something went wrong, so the ASTImporter tried to emit a diagnostic, but we already called `EndSourceFile` at that point. This patch moves the call to `ResetManager` to before `CommitPersistentDecls`, so if we called `HandleDiagnostic`, we would correctly short-circuit out of it. This seems to have been intended anyway based on this comment: https://github.com/llvm/llvm-project/blob/cecdff92838f3c049548e3445a15d8c9c7a49205/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp#L200-L204 But something must have broken that during a refactor. I'm not 100% sure how best to test this because we need a scenario where moving a type into the scratch AST fails, but the expression itself succeeded. rdar://159647906
This PR is a part of the effort to make the VFS used in the compiler
more explicit and consistent.
Instead of creating the VFS deep within the compiler (in
`CompilerInstance::createFileManager()`), clients are now required to
explicitly call `CompilerInstance::createVirtualFileSystem()` and
provide the base VFS from the outside.
This PR also helps in breaking up the dependency cycle where creating a
properly configured `DiagnosticsEngine` requires a properly configured
VFS, but creating properly configuring a VFS requires the
`DiagnosticsEngine`.
Both `CompilerInstance::create{FileManager,Diagnostics}()` now just use
the VFS already in `CompilerInstance` instead of taking one as a
parameter, making the VFS consistent across the instance sub-object.
This patch moves the `frame_lang` logic to just the logging (because that's what it was always used for anyway). The callsites decide whether to fall back on to the frame language or not when running the expression.