diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -12,6 +12,7 @@ #include "../clang-tidy/ClangTidyOptions.h" #include "CodeComplete.h" #include "ConfigProvider.h" +#include "Diagnostics.h" #include "DraftStore.h" #include "FeatureModule.h" #include "GlobalCompilationDatabase.h" @@ -40,6 +41,7 @@ #include #include #include +#include namespace clang { namespace clangd { @@ -339,6 +341,11 @@ void customAction(PathRef File, llvm::StringRef Name, Callback Action); + /// Fetches diagnostics for current version of the \p File. This might fail if + /// server is busy and would require a long time to prepare diagnostics. It + /// should provide diagnostics eventually. + void pullDiags(PathRef File, Callback> CB); + /// Returns estimated memory usage and other statistics for each of the /// currently open files. /// Overall memory usage of clangd may be significantly more than reported diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -9,6 +9,7 @@ #include "ClangdServer.h" #include "CodeComplete.h" #include "Config.h" +#include "Diagnostics.h" #include "DumpAST.h" #include "FindSymbols.h" #include "Format.h" @@ -81,7 +82,8 @@ if (FIndex) FIndex->updateMain(Path, AST); - std::vector Diagnostics = AST.getDiagnostics(); + // We issue callback only with fresh preambles, so diagnostics should exist. + std::vector Diagnostics = *AST.getDiagnostics(); if (ServerCallbacks) Publish([&]() { ServerCallbacks->onDiagnosticsReady(Path, AST.version(), @@ -902,6 +904,21 @@ WorkScheduler->runWithAST(Name, File, std::move(Action)); } +void ClangdServer::pullDiags(PathRef File, Callback> CB) { + auto Action = [CB = std::move(CB)]( + llvm::Expected InpAST) mutable { + if (!InpAST) + return CB(InpAST.takeError()); + if (auto Diags = InpAST->AST.getDiagnostics()) + return CB(*Diags); + // FIXME: Use ServerCancelled error once it is settled in LSP-3.17. + return CB(llvm::make_error("server is busy building diagnostics", + ErrorCode::InternalError)); + }; + + WorkScheduler->runWithAST("PullDiags", File, std::move(Action)); +} + llvm::StringMap ClangdServer::fileStats() const { return WorkScheduler->fileStats(); } diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -88,7 +88,7 @@ /// (These should be const, but RecursiveASTVisitor requires Decl*). ArrayRef getLocalTopLevelDecls(); - const std::vector &getDiagnostics() const; + const auto &getDiagnostics() const { return Diags; } /// Returns the estimated size of the AST and the accessory structures, in /// bytes. Does not include the size of the preamble. @@ -120,7 +120,7 @@ std::unique_ptr Clang, std::unique_ptr Action, syntax::TokenBuffer Tokens, MainFileMacros Macros, std::vector LocalTopLevelDecls, - std::vector Diags, IncludeStructure Includes, + llvm::Optional> Diags, IncludeStructure Includes, CanonicalIncludes CanonIncludes); std::string Version; @@ -142,8 +142,8 @@ /// All macro definitions and expansions in the main file. MainFileMacros Macros; - // Data, stored after parsing. - std::vector Diags; + // Data, stored after parsing. None if AST was built with a stale preamble. + llvm::Optional> Diags; // Top-level decls inside the current file. Not that this does not include // top-level decls from the preamble. std::vector LocalTopLevelDecls; diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -264,9 +264,10 @@ StoreDiags ASTDiags; llvm::Optional Patch; + bool PatchedPreamble = false; if (Preamble) { Patch = PreamblePatch::create(Filename, Inputs, *Preamble); - Patch->apply(*CI); + PatchedPreamble = Patch->apply(*CI); } auto Clang = prepareCompilerInstance( std::move(CI), PreamblePCH, @@ -441,14 +442,20 @@ // CompilerInstance won't run this callback, do it directly. ASTDiags.EndSourceFile(); - std::vector Diags = CompilerInvocationDiags; - // Add diagnostics from the preamble, if any. - if (Preamble) - Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end()); - // Finally, add diagnostics coming from the AST. - { - std::vector D = ASTDiags.take(CTContext.getPointer()); - Diags.insert(Diags.end(), D.begin(), D.end()); + llvm::Optional> Diags; + // FIXME: Also skip generation of diagnostics alltogether to speed up ast + // builds when we are patching a stale preamble. + if (!PatchedPreamble) { + Diags = CompilerInvocationDiags; + // Add diagnostics from the preamble, if any. + if (Preamble) + Diags->insert(Diags->end(), Preamble->Diags.begin(), + Preamble->Diags.end()); + // Finally, add diagnostics coming from the AST. + { + std::vector D = ASTDiags.take(CTContext.getPointer()); + Diags->insert(Diags->end(), D.begin(), D.end()); + } } return ParsedAST(Inputs.Version, std::move(Preamble), std::move(Clang), std::move(Action), std::move(Tokens), std::move(Macros), @@ -493,14 +500,12 @@ const MainFileMacros &ParsedAST::getMacros() const { return Macros; } -const std::vector &ParsedAST::getDiagnostics() const { return Diags; } - std::size_t ParsedAST::getUsedBytes() const { auto &AST = getASTContext(); // FIXME(ibiryukov): we do not account for the dynamically allocated part of // Message and Fixes inside each diagnostic. - std::size_t Total = - clangd::getUsedBytes(LocalTopLevelDecls) + clangd::getUsedBytes(Diags); + std::size_t Total = clangd::getUsedBytes(LocalTopLevelDecls) + + (Diags ? clangd::getUsedBytes(*Diags) : 0); // FIXME: the rest of the function is almost a direct copy-paste from // libclang's clang_getCXTUResourceUsage. We could share the implementation. @@ -541,8 +546,8 @@ std::unique_ptr Action, syntax::TokenBuffer Tokens, MainFileMacros Macros, std::vector LocalTopLevelDecls, - std::vector Diags, IncludeStructure Includes, - CanonicalIncludes CanonIncludes) + llvm::Optional> Diags, + IncludeStructure Includes, CanonicalIncludes CanonIncludes) : Version(Version), Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Tokens(std::move(Tokens)), Macros(std::move(Macros)), Diags(std::move(Diags)), diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -110,7 +110,8 @@ /// Adjusts CI (which compiles the modified inputs) to be used with the /// baseline preamble. This is done by inserting an artifical include to the /// \p CI that contains new directives calculated in create. - void apply(CompilerInvocation &CI) const; + /// Returns true if \p CI is modified. + bool apply(CompilerInvocation &CI) const; /// Returns #include directives from the \c Modified preamble that were /// resolved using the \c Baseline preamble. This covers the new locations of diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -499,10 +499,10 @@ return PP; } -void PreamblePatch::apply(CompilerInvocation &CI) const { +bool PreamblePatch::apply(CompilerInvocation &CI) const { // No need to map an empty file. if (PatchContents.empty()) - return; + return false; auto &PPOpts = CI.getPreprocessorOpts(); auto PatchBuffer = // we copy here to ensure contents are still valid if CI outlives the @@ -513,6 +513,7 @@ // The patch will be parsed after loading the preamble ast and before parsing // the main file. PPOpts.Includes.push_back(PatchFileName); + return true; } std::vector PreamblePatch::preambleIncludes() const { diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -181,7 +181,7 @@ elog("Failed to build AST"); return false; } - ErrCount += showErrors(llvm::makeArrayRef(AST->getDiagnostics()) + ErrCount += showErrors(llvm::makeArrayRef(*AST->getDiagnostics()) .drop_front(Preamble->Diags.size())); if (Opts.BuildDynamicSymbolIndex) { diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -135,7 +135,7 @@ auto TU = TestTU::withCode(Test.code()); TU.ClangTidyProvider = addTidyChecks("google-explicit-constructor"); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), ElementsAre( // This range spans lines. AllOf(Diag(Test.range("typo"), @@ -173,14 +173,14 @@ TEST(DiagnosticsTest, FlagsMatter) { Annotations Test("[[void]] main() {} // error-ok"); auto TU = TestTU::withCode(Test.code()); - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"), WithFix(Fix(Test.range(), "int", "change 'void' to 'int'"))))); // Same code built as C gets different diagnostics. TU.Filename = "Plain.c"; EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), ElementsAre(AllOf( Diag(Test.range(), "return type of 'main' is not 'int'"), WithFix(Fix(Test.range(), "int", "change return type to 'int'"))))); @@ -192,7 +192,7 @@ )cpp"); auto TU = TestTU::withCode(Test.code()); - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(::testing::AllOf( Diag(Test.range(), "'not-found.h' file not found"), DiagSource(Diag::Clang), DiagName("pp_file_not_found")))); @@ -209,7 +209,7 @@ "hicpp-uppercase-literal-suffix"); // Verify that we filter out the duplicated diagnostic message. EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre(::testing::AllOf( Diag(Test.range(), "floating point literal has suffix 'f', which is not uppercase"), @@ -229,7 +229,7 @@ // The check doesn't handle template instantiations which ends up emitting // duplicated messages, verify that we deduplicate them. EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre(::testing::AllOf( Diag(Test.range(), "floating point literal has suffix 'f', which is not uppercase"), @@ -254,7 +254,7 @@ "modernize-deprecated-headers," "modernize-use-trailing-return-type"); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre( AllOf(Diag(Test.range("deprecated"), "inclusion of deprecated C++ header 'assert.h'; consider " @@ -296,7 +296,7 @@ TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = ""; TU.ClangTidyProvider = addTidyChecks("llvm-include-order"); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"), DiagSource(Diag::ClangTidy), DiagName("llvm-include-order")))); @@ -314,7 +314,7 @@ TestTU TU = TestTU::withCode(Main.code()); TU.HeaderCode = Header.code().str(); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), ElementsAre(AllOf( Diag(Main.range(), "in template: base specifier must name a class"), WithNote(Diag(Header.range(), "error occurred here"), @@ -340,7 +340,7 @@ } } )cpp"; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Main.range(), "in template: " @@ -368,7 +368,7 @@ TestTU TU = TestTU::withCode(Main.code()); TU.ClangTidyProvider = addTidyChecks("modernize-loop-convert"); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre(::testing::AllOf( Diag(Main.range(), "use range-based for loop instead"), DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert")))); @@ -384,14 +384,14 @@ )cpp"); auto TU = TestTU::withCode(Main.code()); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), ElementsAre(Diag(Main.range(), "use of undeclared identifier 'unknown'"), Diag(Main.range("ret"), "void function 'x' should not return a value"))); Config Cfg; Cfg.Diagnostics.Suppress.insert("return-type"); WithContextValue WithCfg(Config::Key, std::move(Cfg)); - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(Diag(Main.range(), "use of undeclared identifier 'unknown'"))); } @@ -413,7 +413,7 @@ TestTU TU = TestTU::withCode(Main.code()); TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division"); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre(::testing::AllOf( Diag(Main.range(), "result of integer division used in a floating " "point context; possible loss of precision"), @@ -431,7 +431,7 @@ TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division", "bugprone-integer-division"); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre(::testing::AllOf( Diag(Main.range(), "result of integer division used in a floating " "point context; possible loss of precision"), @@ -450,7 +450,7 @@ )cpp"); TestTU TU = TestTU::withCode(Source.code()); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), ElementsAre(WithFix(Fix( Source.range(), "somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier", @@ -466,7 +466,7 @@ } )cpp"); TU.Code = std::string(Source.code()); - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(WithFix( Fix(Source.range(), "ident", "change 'ide\\…' to 'ident'")))); } @@ -481,7 +481,7 @@ TestTU TU = TestTU::withCode(Main.code()); TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division", "bugprone-integer-division"); - EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre()); } TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) { @@ -496,7 +496,7 @@ )cpp"); TestTU TU = TestTU::withCode(Main.code()); TU.ClangTidyProvider = addTidyChecks("bugprone-bad-signal-to-kill-thread"); - EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash } TEST(DiagnosticTest, ElseAfterReturnRange) { @@ -513,7 +513,7 @@ TestTU TU = TestTU::withCode(Main.code()); TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return"); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'"))); } @@ -532,7 +532,7 @@ #endif )cpp"); EXPECT_THAT( - TestTU::withCode(Test.code()).build().getDiagnostics(), + *TestTU::withCode(Test.code()).build().getDiagnostics(), ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'"))); } @@ -542,7 +542,7 @@ )cpp"); TU.ExtraArgs.push_back("-Xclang"); TU.ExtraArgs.push_back("-verify"); - EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); } // Recursive main-file include is diagnosed, and doesn't crash. @@ -552,7 +552,7 @@ int symbol; )cpp"); TU.Filename = "foo.h"; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(DiagName("pp_including_mainfile_in_preamble"))); EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1)); } @@ -565,7 +565,7 @@ int symbol; )cpp"); TU.Filename = "foo.h"; - EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1)); } @@ -581,7 +581,7 @@ )cpp"); TU.Filename = "foo.h"; // FIXME: should be no errors here. - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(DiagName("pp_including_mainfile_in_preamble"))); EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1)); } @@ -598,7 +598,7 @@ return $bar[[TEN]]; } )cpp"); - EXPECT_THAT(TestTU::withCode(Test.code()).build().getDiagnostics(), + EXPECT_THAT(*TestTU::withCode(Test.code()).build().getDiagnostics(), ElementsAre(Diag(Test.range("foo"), "cannot initialize return object of type " "'int *' with an rvalue of type 'int'"), @@ -614,7 +614,7 @@ [[Define]](main) // error-ok )cpp"); auto TU = TestTU::withCode(Test.code()); - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"), Not(WithFix(_))))); } @@ -625,7 +625,7 @@ llvm::InitializeAllTargetInfos(); // As in ClangdMain auto TU = TestTU::withCode("void fn() { __asm { cmp cl,64 } }"); TU.ExtraArgs = {"-fms-extensions"}; - EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); } TEST(DiagnosticsTest, ToLSP) { @@ -783,7 +783,7 @@ TU.ExternalIndex = Index.get(); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAreArray( {AllOf(Diag(Test.range("nested"), "incomplete type 'ns::X' named in nested name specifier"), @@ -868,7 +868,7 @@ MemIndex::build(std::move(Slab).build(), RefSlab(), RelationSlab()); TU.ExternalIndex = Index.get(); - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Test.range("base"), "base class has incomplete type"), Diag(Test.range("access"), @@ -901,7 +901,7 @@ TU.ExternalIndex = Index.get(); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre( AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"), DiagName("unknown_typename"), @@ -946,7 +946,7 @@ SymbolWithHeader{"na::nb::X", "unittest:///b.h", "\"b.h\""}}); TU.ExternalIndex = Index.get(); - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre(AllOf( Diag(Test.range("unqualified"), "unknown type name 'X'"), DiagName("unknown_typename"), @@ -967,7 +967,7 @@ TU.ExternalIndex = Index.get(); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'"))); } @@ -1002,7 +1002,7 @@ TU.ExternalIndex = Index.get(); auto Parsed = TU.build(); - for (const auto &D : Parsed.getDiagnostics()) { + for (const auto &D : *Parsed.getDiagnostics()) { if (D.Fixes.size() != 1) { ADD_FAILURE() << "D.Fixes.size() != 1"; continue; @@ -1027,7 +1027,7 @@ TU.ExternalIndex = Index.get(); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre(AllOf( Diag(Test.range(), "no member named 'scope' in namespace 'ns'"), DiagName("no_member"), @@ -1055,7 +1055,7 @@ TU.ExternalIndex = Index.get(); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), UnorderedElementsAre( AllOf( Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; " @@ -1098,7 +1098,7 @@ SymbolWithHeader{"a::X", "unittest:///x.h", "\"x.h\""}); TU.ExternalIndex = Index.get(); - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre(AllOf( Diag(Test.range(), "no type named 'X' in namespace 'a'"), DiagName("typename_nested_not_found"), @@ -1124,7 +1124,7 @@ TU.ExternalIndex = Index.get(); EXPECT_THAT( - TU.build().getDiagnostics(), + *TU.build().getDiagnostics(), ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'"))); } @@ -1135,7 +1135,7 @@ Annotations Header("[[no_type_spec]]; // error-ok"); TestTU TU = TestTU::withCode(Main.code()); TU.AdditionalFiles = {{"a.h", std::string(Header.code())}}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre(AllOf( Diag(Main.range(), "in included file: C++ requires a " "type specifier for all declarations"), @@ -1149,7 +1149,7 @@ TestTU TU = TestTU::withCode(Main.code()); TU.AdditionalFiles = {{"a.h", "#include \"b.h\""}, {"b.h", "no_type_spec; // error-ok"}}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Main.range(), "in included file: C++ requires a " "type specifier for all declarations"))); @@ -1163,7 +1163,7 @@ TestTU TU = TestTU::withCode(Main.code()); TU.AdditionalFiles = {{"a.h", "no_type_spec; // error-ok"}, {"b.h", "no_type_spec; // error-ok"}}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Main.range("a"), "in included file: C++ requires a type " "specifier for all declarations"), @@ -1180,7 +1180,7 @@ TU.AdditionalFiles = { {"a.h", "#include \"b.h\"\n"}, {"b.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre(Diag(Main.range(), "in included file: C++ requires a type " "specifier for all declarations"))); @@ -1198,7 +1198,7 @@ {"a.h", "#include \"c.h\"\n"}, {"b.h", "#include \"c.h\"\n"}, {"c.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Main.range(), "in included file: C++ requires a " "type specifier for all declarations"))); @@ -1227,7 +1227,7 @@ no_type_spec_9; no_type_spec_10; #endif)cpp"}}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Main.range(), "in included file: C++ requires a " "type specifier for all declarations"))); @@ -1242,7 +1242,7 @@ int x = 5/0;)cpp"); TestTU TU = TestTU::withCode(Main.code()); TU.AdditionalFiles = {{"a.h", std::string(Header.code())}}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre(AllOf( Diag(Main.range(), "in included file: C++ requires " "a type specifier for all declarations"), @@ -1260,7 +1260,7 @@ TU.AdditionalFiles = {{"a.h", std::string(Header.code())}}; // promote warnings to errors. TU.ExtraArgs = {"-Werror", "-Wunused"}; - EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); } TEST(DiagsInHeaders, FromNonWrittenSources) { @@ -1273,7 +1273,7 @@ TestTU TU = TestTU::withCode(Main.code()); TU.AdditionalFiles = {{"a.h", std::string(Header.code())}}; TU.ExtraArgs = {"-DFOO=NOOO"}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre(AllOf( Diag(Main.range(), "in included file: use of undeclared identifier 'NOOO'"), @@ -1291,7 +1291,7 @@ X;)cpp"); TestTU TU = TestTU::withCode(Main.code()); TU.AdditionalFiles = {{"a.h", std::string(Header.code())}}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Main.range(), "in included file: use of undeclared " "identifier 'foo'; did you mean 'fo'?"))); @@ -1308,7 +1308,7 @@ X(foo);)cpp"); TestTU TU = TestTU::withCode(Main.code()); TU.AdditionalFiles = {{"a.h", std::string(Header.code())}}; - EXPECT_THAT(TU.build().getDiagnostics(), + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Main.range(), "in included file: use of undeclared " "identifier 'foo'; did you mean 'fo'?"))); @@ -1320,7 +1320,7 @@ TU.AdditionalFiles = {{"a.h", "void main();"}}; // The diagnostic "main must return int" is from the header, we don't attempt // to render it in the main file as there is no written location there. - EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); + EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre()); } TEST(ToLSPDiag, RangeIsInMain) { diff --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp b/clang-tools-extra/clangd/unittests/ModulesTests.cpp --- a/clang-tools-extra/clangd/unittests/ModulesTests.cpp +++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp @@ -61,7 +61,7 @@ header "module.h" } )modulemap"; - EXPECT_TRUE(TU.build().getDiagnostics().empty()); + EXPECT_TRUE(TU.build().getDiagnostics()->empty()); } TEST(Modules, Diagnostic) { diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -511,7 +511,7 @@ auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS), std::move(CI), {}, BaselinePreamble); ASSERT_TRUE(PatchedAST); - EXPECT_TRUE(PatchedAST->getDiagnostics().empty()); + EXPECT_FALSE(PatchedAST->getDiagnostics()); } // Then ensure correctness by making sure includes were seen only once. @@ -526,7 +526,7 @@ auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS), std::move(CI), {}, BaselinePreamble); ASSERT_TRUE(PatchedAST); - EXPECT_TRUE(PatchedAST->getDiagnostics().empty()); + EXPECT_FALSE(PatchedAST->getDiagnostics()); EXPECT_THAT(Includes, ElementsAre(WithFileName(testPath("__preamble_patch__.h")), WithFileName("b.h"), WithFileName("a.h"))); @@ -569,7 +569,7 @@ auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI), {}, EmptyPreamble); ASSERT_TRUE(PatchedAST); - ASSERT_TRUE(PatchedAST->getDiagnostics().empty()); + ASSERT_FALSE(PatchedAST->getDiagnostics()); // Ensure source location information is correct, including resolved paths. EXPECT_THAT(PatchedAST->getIncludeStructure().MainFileIncludes, diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -274,8 +274,12 @@ auto AST = createPatchedAST("", Modified.code()); ASSERT_TRUE(AST); - EXPECT_THAT(AST->getDiagnostics(), - Not(Contains(Field(&Diag::Range, Modified.range())))); + bool SeenMacroRef = false; + for (auto &M : AST->getMacros().MacroRefs) { + for (auto &O : M.getSecond()) + SeenMacroRef |= O.Rng == Modified.range(); + } + EXPECT_TRUE(SeenMacroRef); } } @@ -298,8 +302,6 @@ auto AST = createPatchedAST(Baseline, Modified.code()); ASSERT_TRUE(AST); - EXPECT_THAT(AST->getDiagnostics(), - Not(Contains(Field(&Diag::Range, Modified.range())))); } TEST(PreamblePatchTest, LocateMacroAtWorks) { diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -581,7 +581,7 @@ auto TU = TestTU::withCode(Test.code()); TU.AdditionalFiles["Expand.inc"] = "MACRO\n"; auto AST = TU.build(); - EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty()); + EXPECT_THAT(*AST.getDiagnostics(), ::testing::IsEmpty()); auto T = makeSelectionTree(Case, AST); EXPECT_EQ("BreakStmt", T.commonAncestor()->kind()); diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -121,7 +121,7 @@ class CaptureDiags : public ParsingCallbacks { public: void onMainAST(PathRef File, ParsedAST &AST, PublishFn Publish) override { - reportDiagnostics(File, AST.getDiagnostics(), Publish); + reportDiagnostics(File, *AST.getDiagnostics(), Publish); } void onFailedAST(PathRef File, llvm::StringRef Version, diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -128,7 +128,8 @@ return false; }(); if (!ErrorOk) { - for (const auto &D : AST->getDiagnostics()) + // We always build AST with a fresh preamble in TestTU. + for (const auto &D : *AST->getDiagnostics()) if (D.Severity >= DiagnosticsEngine::Error) { ADD_FAILURE() << "TestTU failed to build (suppress with /*error-ok*/): \n" diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp --- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp @@ -398,7 +398,7 @@ // The compiler should produce a diagnostic for hitting the // template instantiation depth. - ASSERT_TRUE(!AST.getDiagnostics().empty()); + ASSERT_TRUE(!AST.getDiagnostics()->empty()); // Make sure getTypeHierarchy() doesn't get into an infinite recursion. // The parent is reported as "S" because "S<0>" is an invalid instantiation.