Author: Kadir Cetinkaya Date: 2023-06-28T10:41:45+02:00 New Revision: bd89f9ec293ea5e20e89d4bce49135431239cf3e
URL: https://github.com/llvm/llvm-project/commit/bd89f9ec293ea5e20e89d4bce49135431239cf3e DIFF: https://github.com/llvm/llvm-project/commit/bd89f9ec293ea5e20e89d4bce49135431239cf3e.diff LOG: [clangd] Always allow diagnostics from stale preambles We've been running this internally for months now, without any stability or correctness concerns. It has ~40% speed up on incremental diagnostics latencies (as preamble can get invalidated through code completion etc.). Differential Revision: https://reviews.llvm.org/D153882 Added: Modified: clang-tools-extra/clangd/ClangdLSPServer.cpp clang-tools-extra/clangd/ClangdLSPServer.h clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clangd/ClangdServer.h clang-tools-extra/clangd/Config.h clang-tools-extra/clangd/ConfigCompile.cpp clang-tools-extra/clangd/ConfigFragment.h clang-tools-extra/clangd/ConfigYAML.cpp clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/ParsedAST.h clang-tools-extra/clangd/Preamble.cpp clang-tools-extra/clangd/Preamble.h clang-tools-extra/clangd/TUScheduler.cpp clang-tools-extra/clangd/tool/Check.cpp clang-tools-extra/clangd/unittests/ClangdTests.cpp clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp clang-tools-extra/clangd/unittests/ModulesTests.cpp clang-tools-extra/clangd/unittests/ParsedASTTests.cpp clang-tools-extra/clangd/unittests/PreambleTests.cpp clang-tools-extra/clangd/unittests/SelectionTests.cpp clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp clang-tools-extra/clangd/unittests/TestTU.cpp clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 396b903a9a762..bf4c1a982834b 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -650,7 +650,6 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, CodeAction::INFO_KIND}}} : llvm::json::Value(true); - std::vector<llvm::StringRef> Commands; for (llvm::StringRef Command : Handlers.CommandHandlers.keys()) Commands.push_back(Command); @@ -1744,7 +1743,7 @@ bool ClangdLSPServer::shouldRunCompletion( } void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version, - std::vector<Diag> Diagnostics) { + llvm::ArrayRef<Diag> Diagnostics) { PublishDiagnosticsParams Notification; Notification.version = decodeVersion(Version); Notification.uri = URIForFile::canonicalize(File, /*TUPath=*/File); diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index a483d1110f64c..6cf1db4306324 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDLSPSERVER_H #include "ClangdServer.h" +#include "Diagnostics.h" #include "GlobalCompilationDatabase.h" #include "LSPBinder.h" #include "Protocol.h" @@ -18,6 +19,7 @@ #include "support/MemoryTree.h" #include "support/Path.h" #include "support/Threading.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/Support/JSON.h" #include <chrono> #include <cstddef> @@ -80,7 +82,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks, private: // Implement ClangdServer::Callbacks. void onDiagnosticsReady(PathRef File, llvm::StringRef Version, - std::vector<Diag> Diagnostics) override; + llvm::ArrayRef<Diag> Diagnostics) override; void onFileUpdated(PathRef File, const TUStatus &Status) override; void onBackgroundIndexProgress(const BackgroundQueue::Stats &Stats) override; void onSemanticsMaybeChanged(PathRef File) override; @@ -197,7 +199,6 @@ class ClangdLSPServer : private ClangdServer::Callbacks, void bindMethods(LSPBinder &, const ClientCapabilities &Caps); std::vector<CodeAction> getFixes(StringRef File, const clangd::Diagnostic &D); - /// Checks if completion request should be ignored. We need this due to the /// limitation of the LSP. Per LSP, a client sends requests for all "trigger /// character" we specify, but for '>' and ':' we need to check they actually diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 7b4224a20cec3..d8df4c6a69910 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -124,13 +124,10 @@ struct UpdateIndexCallbacks : public ParsingCallbacks { if (FIndex) FIndex->updateMain(Path, AST); - assert(AST.getDiagnostics() && - "We issue callback only with fresh preambles"); - std::vector<Diag> Diagnostics = *AST.getDiagnostics(); if (ServerCallbacks) Publish([&]() { ServerCallbacks->onDiagnosticsReady(Path, AST.version(), - std::move(Diagnostics)); + AST.getDiagnostics()); if (CollectInactiveRegions) { ServerCallbacks->onInactiveRegionsReady(Path, getInactiveRegions(AST)); @@ -366,7 +363,7 @@ ClangdServer::createConfiguredContextProvider(const config::Provider *Provider, std::lock_guard<std::mutex> Lock(PublishMu); for (auto &Entry : ReportableDiagnostics) Publish->onDiagnosticsReady(Entry.first(), /*Version=*/"", - std::move(Entry.second)); + Entry.second); } return Context::current().derive(Config::Key, std::move(C)); } @@ -1046,11 +1043,7 @@ void ClangdServer::diagnostics(PathRef File, Callback<std::vector<Diag>> CB) { [CB = std::move(CB)](llvm::Expected<InputsAndAST> 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<LSPError>("server is busy parsing includes", - ErrorCode::InternalError)); + return CB(InpAST->AST.getDiagnostics()); }; WorkScheduler->runWithAST("Diagnostics", File, std::move(Action)); diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index d203595d76c7f..ea35b34da4ad6 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -30,6 +30,7 @@ #include "support/Path.h" #include "support/ThreadsafeFS.h" #include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/StringRef.h" #include <functional> @@ -67,7 +68,7 @@ class ClangdServer { /// file, they do not interfere with "pull-based" ClangdServer::diagnostics. /// May be called concurrently for separate files, not for a single file. virtual void onDiagnosticsReady(PathRef File, llvm::StringRef Version, - std::vector<Diag> Diagnostics) {} + llvm::ArrayRef<Diag> Diagnostics) {} /// Called whenever the file status is updated. /// May be called concurrently for separate files, not for a single file. virtual void onFileUpdated(PathRef File, const TUStatus &Status) {} diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index 78fcec1582f00..502dd01df1483 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -105,9 +105,6 @@ struct Config { llvm::StringMap<std::string> CheckOptions; } ClangTidy; - /// Enable emitting diagnostics using stale preambles. - bool AllowStalePreamble = false; - IncludesPolicy UnusedIncludes = IncludesPolicy::Strict; IncludesPolicy MissingIncludes = IncludesPolicy::None; diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index ac158e2ffdd1d..64d6539907233 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -451,13 +451,6 @@ struct FragmentCompiler { } } - if (F.AllowStalePreamble) { - if (auto Val = F.AllowStalePreamble) - Out.Apply.push_back([Val](const Params &, Config &C) { - C.Diagnostics.AllowStalePreamble = **Val; - }); - } - if (F.MissingIncludes) if (auto Val = compileEnum<Config::IncludesPolicy>("MissingIncludes", **F.MissingIncludes) diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index fe2f68f5c45bc..0300f7a5cb73f 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -235,9 +235,6 @@ struct Fragment { /// - std::nullopt std::optional<Located<std::string>> UnusedIncludes; - /// Enable emitting diagnostics using stale preambles. - std::optional<Located<bool>> AllowStalePreamble; - /// Controls if clangd should analyze missing #include directives. /// clangd will warn if no header providing a symbol is `#include`d /// (missing) directly, and suggest adding it. diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index 4d91062d890fe..371c9d3ed4d52 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -134,9 +134,6 @@ class Parser { }); Dict.handle("Includes", [&](Node &N) { parse(F.Includes, N); }); Dict.handle("ClangTidy", [&](Node &N) { parse(F.ClangTidy, N); }); - Dict.handle("AllowStalePreamble", [&](Node &N) { - F.AllowStalePreamble = boolValue(N, "AllowStalePreamble"); - }); Dict.parse(N); } diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 4e94dfed81423..b810feab07581 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -422,7 +422,6 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, }); std::optional<PreamblePatch> Patch; - bool PreserveDiags = true; // We might use an ignoring diagnostic consumer if they are going to be // dropped later on to not pay for extra latency by processing them. DiagnosticConsumer *DiagConsumer = &ASTDiags; @@ -430,9 +429,6 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, if (Preamble) { Patch = PreamblePatch::createFullPatch(Filename, Inputs, *Preamble); Patch->apply(*CI); - PreserveDiags = Patch->preserveDiagnostics(); - if (!PreserveDiags) - DiagConsumer = &DropDiags; } auto Clang = prepareCompilerInstance( std::move(CI), PreamblePCH, @@ -448,7 +444,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, return std::nullopt; } tidy::ClangTidyOptions ClangTidyOpts; - if (PreserveDiags) { + { trace::Span Tracer("ClangTidyOpts"); ClangTidyOpts = getTidyOptionsForFile(Inputs.ClangTidyProvider, Filename); dlog("ClangTidy configuration for file {0}: {1}", Filename, @@ -479,9 +475,6 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, applyWarningOptions(*ClangTidyOpts.ExtraArgsBefore, TidyGroups, Diags); if (ClangTidyOpts.ExtraArgs) applyWarningOptions(*ClangTidyOpts.ExtraArgs, TidyGroups, Diags); - } else { - // Skips some analysis. - Clang->getDiagnosticOpts().IgnoreWarnings = true; } auto Action = std::make_unique<ClangdFrontendAction>(); @@ -517,7 +510,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, llvm::DenseMap<diag::kind, DiagnosticsEngine::Level> OverriddenSeverity; // No need to run clang-tidy or IncludeFixerif we are not going to surface // diagnostics. - if (PreserveDiags) { + { trace::Span Tracer("ClangTidyInit"); static const auto *CTFactories = [] { auto *CTFactories = new tidy::ClangTidyCheckFactories; @@ -712,28 +705,24 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, // CompilerInstance won't run this callback, do it directly. ASTDiags.EndSourceFile(); - std::optional<std::vector<Diag>> Diags; + std::vector<Diag> Diags = CompilerInvocationDiags; // FIXME: Also skip generation of diagnostics alltogether to speed up ast // builds when we are patching a stale preamble. - if (PreserveDiags) { - Diags = CompilerInvocationDiags; - // Add diagnostics from the preamble, if any. - if (Preamble) - llvm::append_range(*Diags, Patch->patchedDiags()); - // Finally, add diagnostics coming from the AST. - { - std::vector<Diag> D = ASTDiags.take(&*CTContext); - Diags->insert(Diags->end(), D.begin(), D.end()); - } + // Add diagnostics from the preamble, if any. + if (Preamble) + llvm::append_range(Diags, Patch->patchedDiags()); + // Finally, add diagnostics coming from the AST. + { + std::vector<Diag> D = ASTDiags.take(&*CTContext); + Diags.insert(Diags.end(), D.begin(), D.end()); } ParsedAST Result(Filename, Inputs.Version, std::move(Preamble), std::move(Clang), std::move(Action), std::move(Tokens), std::move(Macros), std::move(Marks), std::move(ParsedDecls), std::move(Diags), std::move(Includes), std::move(CanonIncludes)); - if (Result.Diags) - llvm::move(getIncludeCleanerDiags(Result, Inputs.Contents), - std::back_inserter(*Result.Diags)); + llvm::move(getIncludeCleanerDiags(Result, Inputs.Contents), + std::back_inserter(Result.Diags)); return std::move(Result); } @@ -785,8 +774,8 @@ 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) + - (Diags ? clangd::getUsedBytes(*Diags) : 0); + std::size_t Total = + clangd::getUsedBytes(LocalTopLevelDecls) + clangd::getUsedBytes(Diags); // FIXME: the rest of the function is almost a direct copy-paste from // libclang's clang_getCXTUResourceUsage. We could share the implementation. @@ -828,8 +817,8 @@ ParsedAST::ParsedAST(PathRef TUPath, llvm::StringRef Version, syntax::TokenBuffer Tokens, MainFileMacros Macros, std::vector<PragmaMark> Marks, std::vector<Decl *> LocalTopLevelDecls, - std::optional<std::vector<Diag>> Diags, - IncludeStructure Includes, CanonicalIncludes CanonIncludes) + std::vector<Diag> Diags, IncludeStructure Includes, + CanonicalIncludes CanonIncludes) : TUPath(TUPath), Version(Version), Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Tokens(std::move(Tokens)), Macros(std::move(Macros)), @@ -853,5 +842,6 @@ std::optional<llvm::StringRef> ParsedAST::preambleVersion() const { return llvm::StringRef(Preamble->Version); } +llvm::ArrayRef<Diag> ParsedAST::getDiagnostics() const { return Diags; } } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h index 27fa7e6ddd872..307100ae0fc49 100644 --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -89,9 +89,7 @@ class ParsedAST { ArrayRef<Decl *> getLocalTopLevelDecls(); ArrayRef<const Decl *> getLocalTopLevelDecls() const; - const std::optional<std::vector<Diag>> &getDiagnostics() const { - return Diags; - } + llvm::ArrayRef<Diag> getDiagnostics() const; /// Returns the estimated size of the AST and the accessory structures, in /// bytes. Does not include the size of the preamble. @@ -131,9 +129,8 @@ class ParsedAST { std::unique_ptr<CompilerInstance> Clang, std::unique_ptr<FrontendAction> Action, syntax::TokenBuffer Tokens, MainFileMacros Macros, std::vector<PragmaMark> Marks, - std::vector<Decl *> LocalTopLevelDecls, - std::optional<std::vector<Diag>> Diags, IncludeStructure Includes, - CanonicalIncludes CanonIncludes); + std::vector<Decl *> LocalTopLevelDecls, std::vector<Diag> Diags, + IncludeStructure Includes, CanonicalIncludes CanonIncludes); Path TUPath; std::string Version; @@ -157,9 +154,9 @@ class ParsedAST { MainFileMacros Macros; // Pragma marks in the main file. std::vector<PragmaMark> Marks; - // Data, stored after parsing. std::nullopt if AST was built with a stale - // preamble. - std::optional<std::vector<Diag>> Diags; + // Diags emitted while parsing this AST (including preamble and compiler + // invocation). + std::vector<Diag> Diags; // Top-level decls inside the current file. Not that this does not include // top-level decls from the preamble. std::vector<Decl *> LocalTopLevelDecls; diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index 878267413e26f..212f882d313ce 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -933,10 +933,6 @@ PreamblePatch PreamblePatch::unmodified(const PreambleData &Preamble) { return PP; } -bool PreamblePatch::preserveDiagnostics() const { - return PatchContents.empty() || - Config::current().Diagnostics.AllowStalePreamble; -} llvm::ArrayRef<PragmaMark> PreamblePatch::marks() const { if (PatchContents.empty()) return Baseline->Marks; diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index 56ba6bc2fb637..b3e53909ac73c 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -204,9 +204,6 @@ class PreamblePatch { /// Returns textual patch contents. llvm::StringRef text() const { return PatchContents; } - /// Whether diagnostics generated using this patch are trustable. - bool preserveDiagnostics() const; - /// Returns diag locations for Modified contents. llvm::ArrayRef<Diag> patchedDiags() const { return PatchedDiags; } diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index 3922c5dda2431..968f4452e020e 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -944,8 +944,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags, // rebuild. Newly built preamble cannot emit diagnostics before this call // finishes (ast callbacks are called from astpeer thread), hence we // gurantee eventual consistency. - if (LatestPreamble && WantDiags != WantDiagnostics::No && - Config::current().Diagnostics.AllowStalePreamble) + if (LatestPreamble && WantDiags != WantDiagnostics::No) generateDiagnostics(std::move(Invocation), std::move(Inputs), std::move(CompilerInvocationDiags)); @@ -1101,7 +1100,7 @@ void ASTWorker::updatePreamble(std::unique_ptr<CompilerInvocation> CI, llvm::StringLiteral TaskName = "Build AST"; // Store preamble and build diagnostics with new preamble if requested. auto Task = [this, Preamble = std::move(Preamble), CI = std::move(CI), - PI = std::move(PI), CIDiags = std::move(CIDiags), + CIDiags = std::move(CIDiags), WantDiags = std::move(WantDiags)]() mutable { // Update the preamble inside ASTWorker queue to ensure atomicity. As a task // running inside ASTWorker assumes internals won't change until it @@ -1127,22 +1126,17 @@ void ASTWorker::updatePreamble(std::unique_ptr<CompilerInvocation> CI, // We only need to build the AST if diagnostics were requested. if (WantDiags == WantDiagnostics::No) return; - // The file may have been edited since we started building this preamble. - // If diagnostics need a fresh preamble, we must use the old version that - // matches the preamble. We make forward progress as updatePreamble() - // receives increasing versions, and this is the only place we emit - // diagnostics. - // If diagnostics can use a stale preamble, we use the current contents of - // the file instead. This provides more up-to-date diagnostics, and avoids - // diagnostics going backwards (we may have already emitted staler-preamble - // diagnostics for the new version). We still have eventual consistency: at - // some point updatePreamble() will catch up to the current file. - if (Config::current().Diagnostics.AllowStalePreamble) - PI = FileInputs; + // Since the file may have been edited since we started building this + // preamble, we use the current contents of the file instead. This provides + // more up-to-date diagnostics, and avoids diagnostics going backwards (we + // may have already emitted staler-preamble diagnostics for the new + // version). + // We still have eventual consistency: at some point updatePreamble() will + // catch up to the current file. // Report diagnostics with the new preamble to ensure progress. Otherwise // diagnostics might get stale indefinitely if user keeps invalidating the // preamble. - generateDiagnostics(std::move(CI), std::move(PI), std::move(CIDiags)); + generateDiagnostics(std::move(CI), FileInputs, std::move(CIDiags)); }; if (RunSync) { runTask(TaskName, Task); diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp index 7a65384262556..3a6f4453fa7e7 100644 --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -251,8 +251,8 @@ class Checker { elog("Failed to build AST"); return false; } - ErrCount += showErrors(llvm::ArrayRef(*AST->getDiagnostics()) - .drop_front(Preamble->Diags.size())); + ErrCount += + showErrors(AST->getDiagnostics().drop_front(Preamble->Diags.size())); if (Opts.BuildDynamicSymbolIndex) { log("Indexing AST..."); diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp index be6c2fba12d1c..864337b98f446 100644 --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -25,6 +25,7 @@ #include "clang/Sema/CodeCompleteConsumer.h" #include "clang/Tooling/ArgumentsAdjusters.h" #include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" @@ -75,7 +76,7 @@ bool diagsContainErrors(const std::vector<Diag> &Diagnostics) { class ErrorCheckingCallbacks : public ClangdServer::Callbacks { public: void onDiagnosticsReady(PathRef File, llvm::StringRef Version, - std::vector<Diag> Diagnostics) override { + llvm::ArrayRef<Diag> Diagnostics) override { bool HadError = diagsContainErrors(Diagnostics); std::lock_guard<std::mutex> Lock(Mutex); HadErrorInLastDiags = HadError; @@ -96,7 +97,7 @@ class ErrorCheckingCallbacks : public ClangdServer::Callbacks { class MultipleErrorCheckingCallbacks : public ClangdServer::Callbacks { public: void onDiagnosticsReady(PathRef File, llvm::StringRef Version, - std::vector<Diag> Diagnostics) override { + llvm::ArrayRef<Diag> Diagnostics) override { bool HadError = diagsContainErrors(Diagnostics); std::lock_guard<std::mutex> Lock(Mutex); @@ -305,7 +306,7 @@ TEST(ClangdServerTest, PropagatesContexts) { } FS; struct Callbacks : public ClangdServer::Callbacks { void onDiagnosticsReady(PathRef File, llvm::StringRef Version, - std::vector<Diag> Diagnostics) override { + llvm::ArrayRef<Diag> Diagnostics) override { Got = Context::current().getExisting(Secret); } int Got; @@ -371,7 +372,7 @@ TEST(ClangdServerTest, PropagatesVersion) { MockFS FS; struct Callbacks : public ClangdServer::Callbacks { void onDiagnosticsReady(PathRef File, llvm::StringRef Version, - std::vector<Diag> Diagnostics) override { + llvm::ArrayRef<Diag> Diagnostics) override { Got = Version.str(); } std::string Got = ""; @@ -688,7 +689,7 @@ int d; TestDiagConsumer() : Stats(FilesCount, FileStat()) {} void onDiagnosticsReady(PathRef File, llvm::StringRef Version, - std::vector<Diag> Diagnostics) override { + llvm::ArrayRef<Diag> Diagnostics) override { StringRef FileIndexStr = llvm::sys::path::stem(File); ASSERT_TRUE(FileIndexStr.consume_front("Foo")); @@ -867,7 +868,7 @@ TEST(ClangdThreadingTest, NoConcurrentDiagnostics) { : StartSecondReparse(std::move(StartSecondReparse)) {} void onDiagnosticsReady(PathRef, llvm::StringRef, - std::vector<Diag>) override { + llvm::ArrayRef<Diag>) override { ++Count; std::unique_lock<std::mutex> Lock(Mutex, std::try_to_lock_t()); ASSERT_TRUE(Lock.owns_lock()) @@ -1204,7 +1205,7 @@ TEST(ClangdServer, TidyOverrideTest) { struct DiagsCheckingCallback : public ClangdServer::Callbacks { public: void onDiagnosticsReady(PathRef File, llvm::StringRef Version, - std::vector<Diag> Diagnostics) override { + llvm::ArrayRef<Diag> Diagnostics) override { std::lock_guard<std::mutex> Lock(Mutex); HadDiagsInLastCallback = !Diagnostics.empty(); } diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp index f0ba582187fd5..f0ffc429c0ca9 100644 --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -540,21 +540,6 @@ TEST_F(ConfigCompileTests, Style) { EXPECT_TRUE(compileAndApply()); EXPECT_THAT(Conf.Style.FullyQualifiedNamespaces, ElementsAre("foo", "bar")); } - -TEST_F(ConfigCompileTests, AllowDiagsFromStalePreamble) { - Frag = {}; - EXPECT_TRUE(compileAndApply()); - // Off by default. - EXPECT_EQ(Conf.Diagnostics.AllowStalePreamble, false); - - Frag.Diagnostics.AllowStalePreamble.emplace(true); - EXPECT_TRUE(compileAndApply()); - EXPECT_EQ(Conf.Diagnostics.AllowStalePreamble, true); - - Frag.Diagnostics.AllowStalePreamble.emplace(false); - EXPECT_TRUE(compileAndApply()); - EXPECT_EQ(Conf.Diagnostics.AllowStalePreamble, false); -} } // namespace } // namespace config } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp index dda81ec78aa6b..44a6647d4c0a8 100644 --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -290,33 +290,6 @@ TEST(ParseYAML, Style) { EXPECT_THAT(Results[0].Style.FullyQualifiedNamespaces, ElementsAre(val("foo"), val("bar"))); } - -TEST(ParseYAML, DiagnosticsMode) { - CapturedDiags Diags; - { - Annotations YAML(R"yaml( -Diagnostics: - AllowStalePreamble: Yes)yaml"); - auto Results = - Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback()); - ASSERT_THAT(Diags.Diagnostics, IsEmpty()); - ASSERT_EQ(Results.size(), 1u); - EXPECT_THAT(Results[0].Diagnostics.AllowStalePreamble, - llvm::ValueIs(val(true))); - } - - { - Annotations YAML(R"yaml( -Diagnostics: - AllowStalePreamble: No)yaml"); - auto Results = - Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback()); - ASSERT_THAT(Diags.Diagnostics, IsEmpty()); - ASSERT_EQ(Results.size(), 1u); - EXPECT_THAT(Results[0].Diagnostics.AllowStalePreamble, - llvm::ValueIs(val(false))); - } -} } // namespace } // namespace config } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 62b5c466ee77c..5f36f28becb0d 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -175,7 +175,7 @@ o]](); )cpp"); auto TU = TestTU::withCode(Test.code()); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ElementsAre( // Make sure the whole token is highlighted. AllOf(Diag(Test.range("range"), @@ -224,7 +224,7 @@ TEST(DiagnosticsTest, WSwitch) { )cpp"); auto TU = TestTU::withCode(Test.code()); TU.ExtraArgs = {"-Wswitch"}; - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), ElementsAre(Diag(Test.range(), "enumeration value 'X' not handled in switch"))); } @@ -232,14 +232,14 @@ TEST(DiagnosticsTest, WSwitch) { 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 diff erent 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'"))))); @@ -251,7 +251,7 @@ TEST(DiagnosticsTest, DiagnosticPreamble) { )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")))); @@ -268,7 +268,7 @@ TEST(DiagnosticsTest, DeduplicatedClangTidyDiagnostics) { "hicpp-uppercase-literal-suffix"); // Verify that we filter out the duplicated diagnostic message. EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Test.range(), "floating point literal has suffix 'f', which is not uppercase"), @@ -288,7 +288,7 @@ TEST(DiagnosticsTest, DeduplicatedClangTidyDiagnostics) { // 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(), ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Test.range(), "floating point literal has suffix 'f', which is not uppercase"), @@ -324,7 +324,7 @@ TEST(DiagnosticsTest, ClangTidy) { "misc-no-recursion"); TU.ExtraArgs.push_back("-Wno-unsequenced"); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ifTidyChecks(UnorderedElementsAre( AllOf(Diag(Test.range("deprecated"), "inclusion of deprecated C++ header 'assert.h'; consider " @@ -367,7 +367,7 @@ TEST(DiagnosticsTest, ClangTidyEOF) { TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = ""; TU.ClangTidyProvider = addTidyChecks("llvm-include-order"); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ifTidyChecks(Contains( AllOf(Diag(Test.range(), "#includes are not sorted properly"), diagSource(Diag::ClangTidy), diagName("llvm-include-order"))))); @@ -385,7 +385,7 @@ TEST(DiagnosticTest, TemplatesInHeaders) { 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"), @@ -411,7 +411,7 @@ TEST(DiagnosticTest, MakeUnique) { } } )cpp"; - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Main.range(), "in template: " @@ -437,7 +437,7 @@ TEST(DiagnosticTest, MakeShared) { } )cpp"; TU.ParseOpts.PreambleParseForwardingFunctions = true; - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre( Diag(Main.range(), "in template: " @@ -465,7 +465,7 @@ TEST(DiagnosticTest, NoMultipleDiagnosticInFlight) { TestTU TU = TestTU::withCode(Main.code()); TU.ClangTidyProvider = addTidyChecks("modernize-loop-convert"); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Main.range(), "use range-based for loop instead"), diagSource(Diag::ClangTidy), diagName("modernize-loop-convert"))))); @@ -481,14 +481,14 @@ TEST(DiagnosticTest, RespectsDiagnosticConfig) { )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'"))); } @@ -505,7 +505,7 @@ TEST(DiagnosticTest, RespectsDiagnosticConfigInHeader) { Config Cfg; Cfg.Diagnostics.Suppress.insert("init_conversion_failed"); WithContextValue WithCfg(Config::Key, std::move(Cfg)); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); } TEST(DiagnosticTest, ClangTidySuppressionComment) { @@ -532,7 +532,7 @@ TEST(DiagnosticTest, ClangTidySuppressionComment) { TestTU TU = TestTU::withCode(Main.code()); TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division"); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Main.range(), "result of integer division used in a floating " "point context; possible loss of precision"), @@ -565,7 +565,7 @@ TEST(DiagnosticTest, ClangTidySystemMacro) { // Expect to see warning from user macros, but not system macros. // This matches clang-tidy --system-headers=0 (the default). - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), ifTidyChecks( UnorderedElementsAre(Diag(Main.range("inline"), BadDivision), Diag(Main.range("user"), BadDivision)))); @@ -582,7 +582,7 @@ TEST(DiagnosticTest, ClangTidyWarningAsError) { TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division", "bugprone-integer-division"); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Main.range(), "result of integer division used in a floating " "point context; possible loss of precision"), @@ -615,54 +615,54 @@ TEST(DiagnosticTest, ClangTidyEnablesClangWarning) { diagSeverity(DiagnosticsEngine::Warning)); // Check the -Wunused warning isn't initially on. - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); // We enable warnings based on clang-tidy extra args, if the matching // clang-diagnostic- is there. TU.ClangTidyProvider = addClangArgs({"-Wunused"}, "clang-diagnostic-unused-function"); - EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(UnusedFooWarning)); + EXPECT_THAT(TU.build().getDiagnostics(), ElementsAre(UnusedFooWarning)); // clang-diagnostic-* is acceptable TU.ClangTidyProvider = addClangArgs({"-Wunused"}, "clang-diagnostic-*"); - EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(UnusedFooWarning)); + EXPECT_THAT(TU.build().getDiagnostics(), ElementsAre(UnusedFooWarning)); // And plain * (may turn on other checks too). TU.ClangTidyProvider = addClangArgs({"-Wunused"}, "*"); - EXPECT_THAT(*TU.build().getDiagnostics(), Contains(UnusedFooWarning)); + EXPECT_THAT(TU.build().getDiagnostics(), Contains(UnusedFooWarning)); // And we can explicitly exclude a category too. TU.ClangTidyProvider = addClangArgs( {"-Wunused"}, "clang-diagnostic-*,-clang-diagnostic-unused-function"); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); // Without the exact check specified, the warnings are not enabled. TU.ClangTidyProvider = addClangArgs({"-Wunused"}, "clang-diagnostic-unused"); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); // We don't respect other args. TU.ClangTidyProvider = addClangArgs({"-Wunused", "-Dfoo=bar"}, "clang-diagnostic-unused-function"); - EXPECT_THAT(*TU.build().getDiagnostics(), ElementsAre(UnusedFooWarning)) + EXPECT_THAT(TU.build().getDiagnostics(), ElementsAre(UnusedFooWarning)) << "Not unused function 'bar'!"; // -Werror doesn't apply to warnings enabled by clang-tidy extra args. TU.ExtraArgs = {"-Werror"}; TU.ClangTidyProvider = addClangArgs({"-Wunused"}, "clang-diagnostic-unused-function"); - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), ElementsAre(diagSeverity(DiagnosticsEngine::Warning))); // But clang-tidy extra args won't *downgrade* errors to warnings either. TU.ExtraArgs = {"-Wunused", "-Werror"}; TU.ClangTidyProvider = addClangArgs({"-Wunused"}, "clang-diagnostic-unused-function"); - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), ElementsAre(diagSeverity(DiagnosticsEngine::Error))); // FIXME: we're erroneously downgrading the whole group, this should be Error. TU.ExtraArgs = {"-Wunused-function", "-Werror"}; TU.ClangTidyProvider = addClangArgs({"-Wunused"}, "clang-diagnostic-unused-label"); - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), ElementsAre(diagSeverity(DiagnosticsEngine::Warning))); // This looks silly, but it's the typical result if a warning is enabled by a @@ -670,26 +670,26 @@ TEST(DiagnosticTest, ClangTidyEnablesClangWarning) { TU.ExtraArgs = {}; TU.ClangTidyProvider = addClangArgs({"-Wunused", "-Wno-unused"}, "clang-diagnostic-unused-function"); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); // Overriding only works in the proper order. TU.ClangTidyProvider = addClangArgs({"-Wunused"}, {"clang-diagnostic-unused-function"}); - EXPECT_THAT(*TU.build().getDiagnostics(), SizeIs(1)); + EXPECT_THAT(TU.build().getDiagnostics(), SizeIs(1)); // More specific vs less-specific: match clang behavior TU.ClangTidyProvider = addClangArgs({"-Wunused", "-Wno-unused-function"}, {"clang-diagnostic-unused-function"}); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); TU.ClangTidyProvider = addClangArgs({"-Wunused-function", "-Wno-unused"}, {"clang-diagnostic-unused-function"}); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); // We do allow clang-tidy config to disable warnings from the compile // command. It's unclear this is ideal, but it's hard to avoid. TU.ExtraArgs = {"-Wunused"}; TU.ClangTidyProvider = addClangArgs({"-Wno-unused"}, {}); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); } TEST(DiagnosticTest, LongFixMessages) { @@ -703,7 +703,7 @@ TEST(DiagnosticTest, LongFixMessages) { )cpp"); TestTU TU = TestTU::withCode(Source.code()); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ElementsAre(withFix(Fix( Source.range(), "somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier", @@ -719,7 +719,7 @@ n]] = 10; // error-ok } )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'")))); } @@ -729,7 +729,7 @@ TEST(DiagnosticTest, NewLineFixMessage) { TestTU TU = TestTU::withCode(Source.code()); TU.ExtraArgs = {"-Wnewline-eof"}; EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ElementsAre(withFix((Fix(Source.range(), "\n", "insert '\\n'"))))); } @@ -743,7 +743,7 @@ TEST(DiagnosticTest, ClangTidySuppressionCommentTrumpsWarningAsError) { 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) { @@ -758,7 +758,7 @@ TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) { )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) { @@ -774,7 +774,7 @@ TEST(DiagnosticTest, ElseAfterReturnRange) { )cpp"); TestTU TU = TestTU::withCode(Main.code()); TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return"); - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), ifTidyChecks(ElementsAre( Diag(Main.range(), "do not use 'else' after 'return'")))); } @@ -826,7 +826,7 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiags) { ExpectedDFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include <math.h>\n\n"}); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ifTidyChecks(UnorderedElementsAre( AllOf(Diag(Main.range("A"), "'A' should be initialized in a member " "initializer of the constructor"), @@ -855,7 +855,7 @@ TEST(DiagnosticsTest, Preprocessor) { #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'"))); } @@ -865,7 +865,7 @@ TEST(DiagnosticsTest, IgnoreVerify) { )cpp"); TU.ExtraArgs.push_back("-Xclang"); TU.ExtraArgs.push_back("-verify"); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); } TEST(DiagnosticTest, IgnoreBEFilelistOptions) { @@ -876,7 +876,7 @@ TEST(DiagnosticTest, IgnoreBEFilelistOptions) { "-fxray-always-instrument=null", "-fxray-never-instrument=null", "-fxray-attr-list=null"}) { TU.ExtraArgs.push_back(DisableOption); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); TU.ExtraArgs.pop_back(); } } @@ -888,7 +888,7 @@ TEST(DiagnosticsTest, RecursivePreamble) { 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)); } @@ -901,7 +901,7 @@ TEST(DiagnosticsTest, RecursivePreamblePragmaOnce) { int symbol; )cpp"); TU.Filename = "foo.h"; - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), Not(Contains(diagName("pp_including_mainfile_in_preamble")))); EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1)); } @@ -918,7 +918,7 @@ TEST(DiagnosticsTest, RecursivePreambleIfndefGuard) { )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)); } @@ -930,7 +930,7 @@ void foo(int *x); #pragma clang assume_nonnull end )cpp"); auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), IsEmpty()); const auto *X = cast<FunctionDecl>(findDecl(AST, "foo")).getParamDecl(0); ASSERT_TRUE(X->getOriginalType()->getNullability() == NullabilityKind::NonNull); @@ -947,7 +947,7 @@ void bar(int *Y); )cpp"); TU.AdditionalFiles = {{"foo.h", std::string(Header.code())}}; auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diagName("pp_eof_in_assume_nonnull"))); const auto *X = cast<FunctionDecl>(findDecl(AST, "foo")).getParamDecl(0); ASSERT_TRUE(X->getOriginalType()->getNullability() == @@ -968,7 +968,7 @@ TEST(DiagnosticsTest, InsideMacros) { 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'"), @@ -984,7 +984,7 @@ TEST(DiagnosticsTest, NoFixItInMacro) { [[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(_))))); } @@ -993,11 +993,11 @@ TEST(DiagnosticsTest, PragmaSystemHeader) { Annotations Test("#pragma clang [[system_header]]\n"); auto TU = TestTU::withCode(Test.code()); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ElementsAre(AllOf( Diag(Test.range(), "#pragma system_header ignored in main file")))); TU.Filename = "TestTU.h"; - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); } TEST(ClangdTest, MSAsm) { @@ -1006,7 +1006,7 @@ TEST(ClangdTest, MSAsm) { 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) { @@ -1177,7 +1177,7 @@ TEST(IncludeFixerTest, IncompleteType) { Annotations Main(Case.second); TU.Code = Main.code().str() + "\n // error-ok"; EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ElementsAre(AllOf(diagName(Case.first), hasRange(Main.range()), withFix(Fix(Range{}, "#include \"x.h\"\n", "Include \"x.h\" for symbol ns::X"))))) @@ -1208,7 +1208,7 @@ TEST(IncludeFixerTest, IncompleteEnum) { for (auto Case : Tests) { Annotations Main(Case.second); TU.Code = Main.code().str() + "\n // error-ok"; - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), Contains(AllOf(diagName(Case.first), hasRange(Main.range()), withFix(Fix(Range{}, "#include \"x.h\"\n", "Include \"x.h\" for symbol X"))))) @@ -1240,7 +1240,7 @@ int main() { 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"), @@ -1274,7 +1274,7 @@ using Type = ns::$template[[Foo]]<int>; 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"), @@ -1325,7 +1325,7 @@ ID(ns::X a6); // FIXME: -fms-compatibility (which is default on windows) breaks the // ns::X cases when the namespace is undeclared. Find out why! TU.ExtraArgs = {"-fno-ms-compatibility"}; - EXPECT_THAT(*TU.build().getDiagnostics(), Each(withFix(_))); + EXPECT_THAT(TU.build().getDiagnostics(), Each(withFix(_))); } TEST(IncludeFixerTest, MultipleMatchedSymbols) { @@ -1344,7 +1344,7 @@ void foo() { 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"), @@ -1365,7 +1365,7 @@ TEST(IncludeFixerTest, NoCrashMemberAccess) { TU.ExternalIndex = Index.get(); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'"))); } @@ -1400,7 +1400,7 @@ void bar(X *x) { 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; @@ -1424,7 +1424,7 @@ void g() { ns::$[[scope]]::X_Y(); } 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"), @@ -1452,7 +1452,7 @@ void f() { TU.ExternalIndex = Index.get(); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), UnorderedElementsAre( AllOf(Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; " "did you mean 'clang'?"), @@ -1492,7 +1492,7 @@ namespace c { 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"), @@ -1518,7 +1518,7 @@ TEST(IncludeFixerTest, NoCrashOnTemplateInstantiations) { TU.ExternalIndex = Index.get(); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'"))); } @@ -1535,7 +1535,7 @@ TEST(IncludeFixerTest, HeaderNamedInDiag) { TU.ExternalIndex = Index.get(); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ElementsAre(AllOf( Diag(Test.range(), "call to undeclared library function 'printf' " "with type 'int (const char *, ...)'; ISO C99 " @@ -1546,7 +1546,7 @@ TEST(IncludeFixerTest, HeaderNamedInDiag) { TU.ExtraArgs = {"-xc", "-std=c89"}; EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ElementsAre(AllOf( Diag(Test.range(), "implicitly declaring library function 'printf' " "with type 'int (const char *, ...)'"), @@ -1572,7 +1572,7 @@ TEST(IncludeFixerTest, CImplicitFunctionDecl) { TU.ExternalIndex = Index.get(); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ElementsAre(AllOf( Diag(Test.range(), "call to undeclared function 'foo'; ISO C99 and later do not " @@ -1581,7 +1581,7 @@ TEST(IncludeFixerTest, CImplicitFunctionDecl) { "Include \"foo.h\" for symbol foo"))))); TU.ExtraArgs = {"-std=c89", "-Wall"}; - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), ElementsAre(AllOf( Diag(Test.range(), "implicit declaration of function 'foo'"), withFix(Fix(Range{}, "#include \"foo.h\"\n", @@ -1595,7 +1595,7 @@ TEST(DiagsInHeaders, DiagInsideHeader) { 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: a type specifier is " "required for all declarations"), @@ -1609,7 +1609,7 @@ TEST(DiagsInHeaders, DiagInTransitiveInclude) { 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: a type specifier is " "required for all declarations"))); @@ -1623,7 +1623,7 @@ TEST(DiagsInHeaders, DiagInMultipleHeaders) { 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: a type specifier is " "required for all declarations"), @@ -1640,7 +1640,7 @@ TEST(DiagsInHeaders, PreferExpansionLocation) { 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(), Contains(Diag(Main.range(), "in included file: a type specifier " "is required for all declarations"))); } @@ -1657,7 +1657,7 @@ TEST(DiagsInHeaders, PreferExpansionLocationMacros) { {"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: a type specifier is " "required for all declarations"))); @@ -1686,7 +1686,7 @@ TEST(DiagsInHeaders, LimitDiagsOutsideMainFile) { 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: a type specifier is " "required for all declarations"))); @@ -1701,7 +1701,7 @@ TEST(DiagsInHeaders, OnlyErrorOrFatal) { 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: a type specifier is " "required for all declarations"), @@ -1719,7 +1719,7 @@ TEST(DiagsInHeaders, OnlyDefaultErrorOrFatal) { 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) { @@ -1732,7 +1732,7 @@ TEST(DiagsInHeaders, FromNonWrittenSources) { 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'"), @@ -1750,7 +1750,7 @@ TEST(DiagsInHeaders, ErrorFromMacroExpansion) { 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'?"))); @@ -1767,7 +1767,7 @@ TEST(DiagsInHeaders, ErrorFromMacroArgument) { 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'?"))); @@ -1779,7 +1779,7 @@ TEST(IgnoreDiags, FromNonWrittenInclude) { 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) { @@ -1827,7 +1827,7 @@ TEST(ParsedASTTest, ModuleSawDiag) { TU.FeatureModules = &FMS; auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), testing::Contains(Diag(Code.range(), KDiagMsg.str()))); } @@ -1838,14 +1838,14 @@ TEST(Preamble, EndsOnNonEmptyLine) { { TU.Code = "#define FOO\n void bar();\n"; auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), IsEmpty()); } { Annotations Code("#define FOO[[]]"); TU.Code = Code.code().str(); auto AST = TU.build(); EXPECT_THAT( - *AST.getDiagnostics(), + AST.getDiagnostics(), testing::Contains(Diag(Code.range(), "no newline at end of file"))); } } @@ -1860,7 +1860,7 @@ TEST(Diagnostics, Tags) { $deprecated[[bar]](); })cpp"); TU.Code = Test.code().str(); - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre( AllOf(Diag(Test.range("unused"), "unused variable 'x'"), withTag(DiagnosticTag::Unnecessary)), @@ -1873,7 +1873,7 @@ TEST(Diagnostics, Tags) { TU.Code = Test.code(); TU.ClangTidyProvider = addTidyChecks("modernize-use-using"); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), ifTidyChecks(UnorderedElementsAre( AllOf(Diag(Test.range("typedef"), "use 'using' instead of 'typedef'"), withTag(DiagnosticTag::Deprecated))))); @@ -1917,22 +1917,22 @@ TEST(DiagnosticsTest, IncludeCleaner) { WithContextValue WithCfg(Config::Key, std::move(Cfg)); auto AST = TU.build(); EXPECT_THAT( - *AST.getDiagnostics(), + AST.getDiagnostics(), Contains(AllOf( Diag(Test.range("diag"), "included header unused.h is not used directly"), withTag(DiagnosticTag::Unnecessary), diagSource(Diag::Clangd), withFix(Fix(Test.range("fix"), "", "remove #include directive"))))); - auto &Diag = AST.getDiagnostics()->front(); + auto &Diag = AST.getDiagnostics().front(); EXPECT_EQ(getDiagnosticDocURI(Diag.Source, Diag.ID, Diag.Name), std::string("https://clangd.llvm.org/guides/include-cleaner";)); Cfg.Diagnostics.SuppressAll = true; WithContextValue SuppressAllWithCfg(Config::Key, std::move(Cfg)); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); Cfg.Diagnostics.SuppressAll = false; Cfg.Diagnostics.Suppress = {"unused-includes"}; WithContextValue SuppressFilterWithCfg(Config::Key, std::move(Cfg)); - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); } TEST(DiagnosticsTest, FixItFromHeader) { @@ -1949,7 +1949,7 @@ TEST(DiagnosticsTest, FixItFromHeader) { TU.Code = Source.code().str(); TU.HeaderCode = Header.str(); EXPECT_THAT( - *TU.build().getDiagnostics(), + TU.build().getDiagnostics(), UnorderedElementsAre(AllOf( Diag(Source.range("diag"), "no matching function for call to 'foo'"), withFix(Fix(Source.range("fix"), "&", @@ -1963,12 +1963,12 @@ TEST(DiagnosticsTest, UnusedInHeader) { auto TU = TestTU::withCode("static inline void foo(void) {}"); TU.ExtraArgs.push_back("-Wunused-function"); TU.Filename = "test.c"; - EXPECT_THAT(*TU.build().getDiagnostics(), + EXPECT_THAT(TU.build().getDiagnostics(), ElementsAre(withID(diag::warn_unused_function))); // Sema should recognize a *.h file open in clangd as a header. // https://github.com/clangd/vscode-clangd/issues/360 TU.Filename = "test.h"; - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); } } // namespace diff --git a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp index ed6a8f7b434ec..2d89c659110b9 100644 --- a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp +++ b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp @@ -76,13 +76,13 @@ TEST(FeatureModulesTest, SuppressDiags) { { auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), testing::Not(testing::IsEmpty())); + EXPECT_THAT(AST.getDiagnostics(), testing::Not(testing::IsEmpty())); } TU.FeatureModules = &FMS; { auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), testing::IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), testing::IsEmpty()); } } @@ -111,13 +111,13 @@ TEST(FeatureModulesTest, BeforeExecute) { { auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), testing::Not(testing::IsEmpty())); + EXPECT_THAT(AST.getDiagnostics(), testing::Not(testing::IsEmpty())); } TU.FeatureModules = &FMS; { auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), testing::IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), testing::IsEmpty()); } } diff --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp b/clang-tools-extra/clangd/unittests/ModulesTests.cpp index acd0fa3111813..5f2fc5d6b8c4b 100644 --- a/clang-tools-extra/clangd/unittests/ModulesTests.cpp +++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp @@ -60,7 +60,7 @@ TEST(Modules, PreambleBuildVisibility) { 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 index 09986b4e0e8a6..b0d5bea1d3279 100644 --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -385,7 +385,6 @@ TEST(ParsedASTTest, PatchesAdditionalIncludes) { auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI), {}, EmptyPreamble); ASSERT_TRUE(PatchedAST); - ASSERT_FALSE(PatchedAST->getDiagnostics()); // Ensure source location information is correct, including resolved paths. EXPECT_THAT(PatchedAST->getIncludeStructure().MainFileIncludes, @@ -533,7 +532,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { ; )cpp"; auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diag("recursively when building a preamble"))); EXPECT_FALSE(mainIsGuarded(AST)); @@ -542,7 +541,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { #include "self.h" // error-ok )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), ElementsAre(diag("nested too deeply"))); + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diag("nested too deeply"))); EXPECT_FALSE(mainIsGuarded(AST)); TU.Code = R"cpp( @@ -551,7 +550,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { ; )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), IsEmpty()); EXPECT_TRUE(mainIsGuarded(AST)); TU.Code = R"cpp( @@ -560,7 +559,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { #include "self.h" )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), IsEmpty()); EXPECT_TRUE(mainIsGuarded(AST)); TU.Code = R"cpp( @@ -569,7 +568,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { #include "self.h" )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), IsEmpty()); EXPECT_TRUE(mainIsGuarded(AST)); TU.Code = R"cpp( @@ -580,7 +579,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { ; )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diag("recursively when building a preamble"))); EXPECT_TRUE(mainIsGuarded(AST)); @@ -592,7 +591,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { #endif )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), IsEmpty()); EXPECT_TRUE(mainIsGuarded(AST)); // Guarded too late... @@ -604,7 +603,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { #endif )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diag("recursively when building a preamble"))); EXPECT_FALSE(mainIsGuarded(AST)); @@ -616,7 +615,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { #endif )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diag("recursively when building a preamble"))); EXPECT_FALSE(mainIsGuarded(AST)); @@ -628,7 +627,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { #endif )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), IsEmpty()); EXPECT_FALSE(mainIsGuarded(AST)); TU.Code = R"cpp( @@ -637,7 +636,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { ; )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diag("recursively when building a preamble"))); EXPECT_TRUE(mainIsGuarded(AST)); @@ -647,7 +646,7 @@ TEST(ParsedASTTest, HeaderGuardsSelfInclude) { #pragma once )cpp"; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diag("recursively when building a preamble"))); EXPECT_TRUE(mainIsGuarded(AST)); } @@ -682,13 +681,13 @@ TEST(ParsedASTTest, HeaderGuardsImplIface) { TU.Code = guard(Interface); TU.AdditionalFiles = {{"impl.h", Implementation}}; auto AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), IsEmpty()); EXPECT_TRUE(mainIsGuarded(AST)); // Slightly harder: the `#pragma once` is part of the preamble, and we // need to transfer it to the main file's HeaderFileInfo. TU.Code = once(Interface); AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST.getDiagnostics(), IsEmpty()); EXPECT_TRUE(mainIsGuarded(AST)); // Editing the implementation file, which is not include guarded. @@ -698,14 +697,14 @@ TEST(ParsedASTTest, HeaderGuardsImplIface) { AST = TU.build(); // The diagnostic is unfortunate in this case, but correct per our model. // Ultimately the include is skipped and the code is parsed correctly though. - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diag("in included file: main file cannot be included " "recursively when building a preamble"))); EXPECT_FALSE(mainIsGuarded(AST)); // Interface is pragma once guarded, same thing. TU.AdditionalFiles = {{"iface.h", once(Interface)}}; AST = TU.build(); - EXPECT_THAT(*AST.getDiagnostics(), + EXPECT_THAT(AST.getDiagnostics(), ElementsAre(diag("in included file: main file cannot be included " "recursively when building a preamble"))); EXPECT_FALSE(mainIsGuarded(AST)); diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp index a370fa82c2f98..03dfa2eaa3386 100644 --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -595,15 +595,6 @@ TEST(PreamblePatch, ModifiedBounds) { } } -TEST(PreamblePatch, DropsDiagnostics) { - llvm::StringLiteral Code = "#define FOO\nx;/* error-ok */"; - // First check that this code generates diagnostics. - EXPECT_THAT(*TestTU::withCode(Code).build().getDiagnostics(), - testing::Not(testing::IsEmpty())); - // Ensure they are dropeed when a patched preamble is used. - EXPECT_FALSE(createPatchedAST("", Code)->getDiagnostics()); -} - TEST(PreamblePatch, MacroLoc) { llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;"; llvm::StringLiteral Modified = " \n#define MACRO 12\nint num = MACRO;"; @@ -638,16 +629,12 @@ MATCHER_P2(Diag, Range, Name, } TEST(PreamblePatch, DiagnosticsFromMainASTAreInRightPlace) { - Config Cfg; - Cfg.Diagnostics.AllowStalePreamble = true; - WithContextValue WithCfg(Config::Key, std::move(Cfg)); - { Annotations Code("#define FOO"); // Check with removals from preamble. Annotations NewCode("[[x]];/* error-ok */"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), + EXPECT_THAT(AST->getDiagnostics(), ElementsAre(Diag(NewCode.range(), "missing_type_specifier"))); } { @@ -658,14 +645,13 @@ TEST(PreamblePatch, DiagnosticsFromMainASTAreInRightPlace) { #define BAR [[x]];/* error-ok */)"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), + EXPECT_THAT(AST->getDiagnostics(), ElementsAre(Diag(NewCode.range(), "missing_type_specifier"))); } } TEST(PreamblePatch, DiagnosticsToPreamble) { Config Cfg; - Cfg.Diagnostics.AllowStalePreamble = true; Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict; Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict; WithContextValue WithCfg(Config::Key, std::move(Cfg)); @@ -680,7 +666,7 @@ TEST(PreamblePatch, DiagnosticsToPreamble) { // Check with removals from preamble. Annotations NewCode(R"([[# include "foo.h"]])"); auto AST = createPatchedAST(Code.code(), NewCode.code(), AdditionalFiles); - EXPECT_THAT(*AST->getDiagnostics(), + EXPECT_THAT(AST->getDiagnostics(), ElementsAre(Diag(NewCode.range(), "unused-includes"))); } { @@ -694,7 +680,7 @@ TEST(PreamblePatch, DiagnosticsToPreamble) { $foo[[#include "foo.h"]])"); auto AST = createPatchedAST(Code.code(), NewCode.code(), AdditionalFiles); EXPECT_THAT( - *AST->getDiagnostics(), + AST->getDiagnostics(), UnorderedElementsAre(Diag(NewCode.range("bar"), "unused-includes"), Diag(NewCode.range("foo"), "unused-includes"))); } @@ -709,17 +695,13 @@ void foo(); #define $foo2[[FOO]] 2)"); auto AST = createPatchedAST(Code.code(), NewCode.code(), AdditionalFiles); EXPECT_THAT( - *AST->getDiagnostics(), + AST->getDiagnostics(), ElementsAre(AllOf(Diag(NewCode.range("foo2"), "-Wmacro-redefined"), withNote(Diag(NewCode.range("foo1")))))); } } TEST(PreamblePatch, TranslatesDiagnosticsInPreamble) { - Config Cfg; - Cfg.Diagnostics.AllowStalePreamble = true; - WithContextValue WithCfg(Config::Key, std::move(Cfg)); - { // Check with additions to preamble. Annotations Code("#include [[<foo>]]"); @@ -727,7 +709,7 @@ TEST(PreamblePatch, TranslatesDiagnosticsInPreamble) { #define BAR #include [[<foo>]])"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), + EXPECT_THAT(AST->getDiagnostics(), ElementsAre(Diag(NewCode.range(), "pp_file_not_found"))); } { @@ -737,7 +719,7 @@ TEST(PreamblePatch, TranslatesDiagnosticsInPreamble) { #include [[<foo>]])"); Annotations NewCode("#include [[<foo>]]"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), + EXPECT_THAT(AST->getDiagnostics(), ElementsAre(Diag(NewCode.range(), "pp_file_not_found"))); } { @@ -745,7 +727,7 @@ TEST(PreamblePatch, TranslatesDiagnosticsInPreamble) { Annotations Code("#include [[<foo>]]"); Annotations NewCode("#define BAR\n#define BAZ\n"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST->getDiagnostics(), IsEmpty()); } { // Picks closest line in case of multiple alternatives. @@ -756,7 +738,7 @@ TEST(PreamblePatch, TranslatesDiagnosticsInPreamble) { #define BAR #include <foo>)"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), + EXPECT_THAT(AST->getDiagnostics(), ElementsAre(Diag(NewCode.range(), "pp_file_not_found"))); } { @@ -764,7 +746,7 @@ TEST(PreamblePatch, TranslatesDiagnosticsInPreamble) { Annotations Code("#include [[<foo>]]"); Annotations NewCode(" # include <foo>"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST->getDiagnostics(), IsEmpty()); } { // Multiple lines. @@ -775,7 +757,7 @@ o>]])"); Annotations NewCode(R"(#include [[<fo\ o>]])"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), + EXPECT_THAT(AST->getDiagnostics(), ElementsAre(Diag(NewCode.range(), "pp_file_not_found"))); } { @@ -788,7 +770,7 @@ o>]])"); Annotations NewCode(R"(#include <fo\ x>)"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST->getDiagnostics(), IsEmpty()); } { // Preserves notes. @@ -802,7 +784,7 @@ x>)"); #define $main[[BAR]] 2)"); auto AST = createPatchedAST(Code.code(), NewCode.code()); EXPECT_THAT( - *AST->getDiagnostics(), + AST->getDiagnostics(), ElementsAre(AllOf(Diag(NewCode.range("main"), "-Wmacro-redefined"), withNote(Diag(NewCode.range("note")))))); } @@ -815,7 +797,7 @@ x>)"); #define $main[[BAR]] 2)"); auto AST = createPatchedAST(Code.code(), NewCode.code()); EXPECT_THAT( - *AST->getDiagnostics(), + AST->getDiagnostics(), ElementsAre(AllOf(Diag(NewCode.range("main"), "-Wmacro-redefined"), Field(&Diag::Notes, IsEmpty())))); } @@ -828,7 +810,7 @@ x>)"); #define BAZ 0 #define BAR 1)"); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST->getDiagnostics(), IsEmpty()); } { Annotations Code(R"( @@ -841,7 +823,7 @@ void foo(); // We shouldn't emit any diagnotiscs (and shouldn't crash). Annotations NewCode(""); auto AST = createPatchedAST(Code.code(), NewCode.code()); - EXPECT_THAT(*AST->getDiagnostics(), IsEmpty()); + EXPECT_THAT(AST->getDiagnostics(), IsEmpty()); } { Annotations Code(R"( @@ -858,7 +840,7 @@ i[[nt]] xyz; )"); auto AST = createPatchedAST(Code.code(), NewCode.code()); EXPECT_THAT( - *AST->getDiagnostics(), + AST->getDiagnostics(), ElementsAre(Diag(NewCode.range(), "pp_unterminated_conditional"))); } } @@ -868,10 +850,6 @@ MATCHER_P2(Mark, Range, Text, "") { } TEST(PreamblePatch, MacroAndMarkHandling) { - Config Cfg; - Cfg.Diagnostics.AllowStalePreamble = true; - WithContextValue WithCfg(Config::Key, std::move(Cfg)); - { Annotations Code(R"cpp( #ifndef FOO diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp index d7ea34c3c7054..422a89a49630d 100644 --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -690,7 +690,7 @@ TEST(SelectionTest, PathologicalPreprocessor) { 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 index cd229d0f2f70f..4e2f0b6434c34 100644 --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -126,7 +126,7 @@ class TUSchedulerTests : public ::testing::Test { 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, @@ -141,9 +141,8 @@ class TUSchedulerTests : public ::testing::Test { if (!D) return; Publish([&]() { - const_cast< - llvm::unique_function<void(PathRef, std::vector<Diag>)> &> (*D)( - File, std::move(Diags)); + const_cast<llvm::unique_function<void(PathRef, std::vector<Diag>)> &>( + *D)(File, Diags); }); } }; @@ -230,20 +229,24 @@ TEST_F(TUSchedulerTests, WantDiagnostics) { Notification Ready; TUScheduler S(CDB, optsForTest(), captureDiags()); auto Path = testPath("foo.cpp"); - updateWithDiags(S, Path, "", WantDiagnostics::Yes, + // Semicolons here and in the following inputs are significant. They ensure + // preamble stays the same across runs. Otherwise we might get multiple + // diagnostics callbacks, once with the stale preamble and another with the + // fresh preamble. + updateWithDiags(S, Path, ";", WantDiagnostics::Yes, [&](std::vector<Diag>) { Ready.wait(); }); - updateWithDiags(S, Path, "request diags", WantDiagnostics::Yes, + updateWithDiags(S, Path, ";request diags", WantDiagnostics::Yes, [&](std::vector<Diag>) { ++CallbackCount; }); - updateWithDiags(S, Path, "auto (clobbered)", WantDiagnostics::Auto, + updateWithDiags(S, Path, ";auto (clobbered)", WantDiagnostics::Auto, [&](std::vector<Diag>) { ADD_FAILURE() << "auto should have been cancelled by auto"; }); - updateWithDiags(S, Path, "request no diags", WantDiagnostics::No, + updateWithDiags(S, Path, ";request no diags", WantDiagnostics::No, [&](std::vector<Diag>) { ADD_FAILURE() << "no diags should not be called back"; }); - updateWithDiags(S, Path, "auto (produces)", WantDiagnostics::Auto, + updateWithDiags(S, Path, ";auto (produces)", WantDiagnostics::Auto, [&](std::vector<Diag>) { ++CallbackCount; }); Ready.notify(); @@ -833,7 +836,9 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) { CDB.ExtraClangFlags.push_back("-DSOMETHING"); ASSERT_TRUE(DoUpdate(SourceContents)); ASSERT_FALSE(DoUpdate(SourceContents)); - ASSERT_EQ(S.fileStats().lookup(Source).ASTBuilds, 4u); + // This causes 2 AST builds always. We first build an AST with the stale + // preamble, and build a second AST once the fresh preamble is ready. + ASSERT_EQ(S.fileStats().lookup(Source).ASTBuilds, 5u); ASSERT_EQ(S.fileStats().lookup(Source).PreambleBuilds, 3u); } @@ -1274,10 +1279,6 @@ TEST_F(TUSchedulerTests, PublishWithStalePreamble) { DiagVersions; }; - Config Cfg; - Cfg.Diagnostics.AllowStalePreamble = true; - WithContextValue WithCfg(Config::Key, std::move(Cfg)); - DiagCollector Collector; Notification UnblockPreamble; auto DiagCallbacks = std::make_unique<BlockPreambleThread>( diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp index 12fbd96cdb0fa..3b6aff7b2c2ac 100644 --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -132,8 +132,6 @@ ParsedAST TestTU::build() const { llvm::errs() << "Failed to build code:\n" << Code; std::abort(); } - assert(AST->getDiagnostics() && - "TestTU should always build an AST with a fresh Preamble"); // Check for error diagnostics and report gtest failures (unless expected). // This guards against accidental syntax errors silently subverting tests. // error-ok is awfully primitive - using clang -verify would be nicer. @@ -150,7 +148,7 @@ ParsedAST TestTU::build() const { }(); if (!ErrorOk) { // We always build AST with a fresh preamble in TestTU. - for (const auto &D : *AST->getDiagnostics()) + for (const auto &D : AST->getDiagnostics()) if (D.Severity >= DiagnosticsEngine::Error) { llvm::errs() << "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 index fba7f3ea2155f..a73ae78766c38 100644 --- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp @@ -402,7 +402,7 @@ TEST(TypeHierarchy, RecursiveHierarchyUnbounded) { // The compiler should produce a diagnostic for hitting the // template instantiation depth. - ASSERT_TRUE(!AST.getDiagnostics()->empty()); + ASSERT_FALSE(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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
