sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Nice! I think we should get to `.clang-tidy` files subsequently, but this is a
great start and allows us to hook up the right set of checks in other
environments.
================
Comment at: clangd/ClangdLSPServer.h:132
- RealFileSystemProvider FSProvider;
/// Options used for code completion
----------------
ilya-biryukov wrote:
> Could we instead call `getRealFS()` when we try to initialize a clang-tidy
> options provider in `main()` and avoid changing this?
> To avoid adding extra non-real-fs "modes of operation" to `ClangdLSPServer`.
> Unless you see other uses for this.
We already have out-of-tree modifications to ClangdLSPServer to use non-real
FSes.
Given that, I think this change is OK... though better still might be to move
it into `ClangdServer::Options`
================
Comment at: clangd/ClangdUnit.cpp:136
auto *ExistingCallbacks = PP.getPPCallbacks();
+ // Don't reply preamble if we don't run any clang-tidy checks without
+ // PPCallbacks.
----------------
I have a hard time understanding this comment.
`No need to replay events if nobody is listening`?
================
Comment at: clangd/ClangdUnit.cpp:274
CTContext.emplace(llvm::make_unique<tidy::DefaultOptionsProvider>(
- tidy::ClangTidyGlobalOptions(), CTOpts));
+ tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
----------------
this is a copy
================
Comment at: clangd/ClangdUnit.cpp:540
llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents),
- PCHs, std::move(VFS));
+ PCHs, std::move(VFS), Inputs.ClangTidyOpts);
}
----------------
this is a copy
================
Comment at: clangd/ClangdUnit.h:83
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+ tidy::ClangTidyOptions ClangTidyOpts);
----------------
Passing by value is OK here if deliberate, but let's try to avoid too many
random copies below.
================
Comment at: clangd/tool/ClangdMain.cpp:204
+static llvm::cl::opt<std::string> ClangTidyChecks(
+ "clang-tidy-checks",
----------------
Maybe add a TODO or FIXME to respect .clang-tidy files?
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55256/new/
https://reviews.llvm.org/D55256
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
