kadircet updated this revision to Diff 275709.
kadircet added a comment.
- Add tests and also disable bugprone-use-after-move
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83224/new/
https://reviews.llvm.org/D83224
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/ClangdTests.cpp
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -23,9 +23,11 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <algorithm>
@@ -1121,6 +1123,42 @@
}
#endif
+TEST(ClangdServer, TidyOverrideTest) {
+ struct DiagsCheckingCallback : public ClangdServer::Callbacks {
+ public:
+ void onDiagnosticsReady(PathRef File, llvm::StringRef Version,
+ std::vector<Diag> Diagnostics) override {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ HadDiagsInLastCallback = !Diagnostics.empty();
+ }
+
+ std::mutex Mutex;
+ bool HadDiagsInLastCallback = false;
+ } DiagConsumer;
+
+ MockFS FS;
+ MockCompilationDatabase CDB;
+ CDB.ExtraClangFlags = {"-xc++"};
+ auto Opts = ClangdServer::optsForTest();
+ Opts.GetClangTidyOptions = [](llvm::vfs::FileSystem &, llvm::StringRef) {
+ auto Opts = tidy::ClangTidyOptions::getDefaults();
+ Opts.Checks = "bugprone-use-after-move,llvm-header-guard";
+ return Opts;
+ };
+ ClangdServer Server(CDB, FS, Opts, &DiagConsumer);
+ const char *SourceContents = R"cpp(
+ struct Foo { Foo(); Foo(Foo&); Foo(Foo&&); };
+ namespace std { Foo&& move(Foo&); }
+ void foo() {
+ Foo x;
+ Foo y = std::move(x);
+ Foo z = x;
+ })cpp";
+ Server.addDocument(testPath("foo.h"), SourceContents);
+ ASSERT_TRUE(Server.blockUntilIdleForTest());
+ EXPECT_FALSE(DiagConsumer.HadDiagsInLastCallback);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -729,23 +729,6 @@
// FIXME: use the FS provided to the function.
Opts = ClangTidyOptProvider->getOptions(File);
}
- if (!Opts.Checks) {
- // If the user hasn't configured clang-tidy checks at all, including
- // via .clang-tidy, give them a nice set of checks.
- // (This should be what the "default" options does, but it isn't...)
- //
- // These default checks are chosen for:
- // - low false-positive rate
- // - providing a lot of value
- // - being reasonably efficient
- Opts.Checks = llvm::join_items(
- ",", "readability-misleading-indentation",
- "readability-deleted-default", "bugprone-integer-division",
- "bugprone-sizeof-expression", "bugprone-suspicious-missing-comma",
- "bugprone-unused-raii", "bugprone-unused-return-value",
- "misc-unused-using-decls", "misc-unused-alias-decls",
- "misc-definitions-in-headers");
- }
return Opts;
};
}
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -39,6 +39,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
@@ -50,6 +51,7 @@
#include <future>
#include <memory>
#include <mutex>
+#include <string>
#include <type_traits>
namespace clang {
@@ -107,6 +109,25 @@
ClangdServer::Callbacks *ServerCallbacks;
bool TheiaSemanticHighlighting;
};
+
+// Set of clang-tidy checks that are not suitable to be run through clangd,
+// either due to crashes or false positives.
+const char *getClangTidyBlacklist() {
+ static const std::string FalsePositives =
+ llvm::join_items(", ",
+ // Check relies on seeing ifndef/define/endif directives,
+ // clangd doesn't replay those when using a preamble.
+ "-llvm-header-guard");
+ static const std::string CrashingChecks =
+ llvm::join_items(", ",
+ // Check can choke on invalid (intermediate) c++ code,
+ // which is often the case when clangd tries to build an
+ // AST.
+ "-bugprone-use-after-move");
+ static const std::string BlackList =
+ llvm::join_items(", ", FalsePositives, CrashingChecks);
+ return BlackList.c_str();
+}
} // namespace
ClangdServer::Options ClangdServer::optsForTest() {
@@ -186,6 +207,28 @@
if (GetClangTidyOptions)
Opts.ClangTidyOpts =
GetClangTidyOptions(*TFS.view(/*CWD=*/llvm::None), File);
+ if (!Opts.ClangTidyOpts.Checks) {
+ // If the user hasn't configured clang-tidy checks at all, including
+ // via .clang-tidy, give them a nice set of checks.
+ // (This should be what the "default" options does, but it isn't...)
+ //
+ // These default checks are chosen for:
+ // - low false-positive rate
+ // - providing a lot of value
+ // - being reasonably efficient
+ Opts.ClangTidyOpts.Checks = llvm::join_items(
+ ",", "readability-misleading-indentation",
+ "readability-deleted-default", "bugprone-integer-division",
+ "bugprone-sizeof-expression", "bugprone-suspicious-missing-comma",
+ "bugprone-unused-raii", "bugprone-unused-return-value",
+ "misc-unused-using-decls", "misc-unused-alias-decls",
+ "misc-definitions-in-headers");
+ } else {
+ // If user has enabled some checks, make sure clangd incompatible ones are
+ // disabled.
+ Opts.ClangTidyOpts.Checks = llvm::join_items(
+ ", ", *Opts.ClangTidyOpts.Checks, getClangTidyBlacklist());
+ }
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
// Compile command is set asynchronously during update, as it can be slow.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
