ilya-biryukov added a comment.
Did a minor rename and added a few `std::move`s before submitting. Was not
worth another round of code review.
Repository:
rL LLVM
https://reviews.llvm.org/D37150
___
cfe-commits mailing list
cfe-commits@lists.llvm
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314678: [clangd] Command line arg to specify
compile_commands.json path (authored by ibiryukov).
Changed prior to commit:
https://reviews.llvm.org/D37150?vs=117340&id=117354#toc
Repository:
rL LLVM
Nebiroth added a comment.
In https://reviews.llvm.org/D37150#885749, @ilya-biryukov wrote:
> Thanks for fixing the last comment.
> Do you want me to land this for you?
Yes please!
https://reviews.llvm.org/D37150
___
cfe-commits mailing list
cfe-
ilya-biryukov added a comment.
Thanks for fixing the last comment.
Do you want me to land this for you?
Comment at: clangd/GlobalCompilationDatabase.cpp:103
+ Logger.log("Failed to find compilation database for " + Twine(File) +
+ "in overriden directory "
Nebiroth updated this revision to Diff 117340.
Nebiroth added a comment.
Improved logging message when unable to find compilation database in specified
overriden directory.
https://reviews.llvm.org/D37150
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCompilation
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
Thanks. LGTM modulo minor NIT (see other inline comment).
Do you need help to land this?
Comment at: clangd/GlobalCompilationDatabase.cpp:102
+if (ReturnValue == nullptr)
+ Logger.log("Failed to find
Nebiroth updated this revision to Diff 117337.
Nebiroth marked 2 inline comments as done.
Nebiroth added a comment.
Changed placement of logging instruction to reduce logging output.
https://reviews.llvm.org/D37150
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCo
ilya-biryukov requested changes to this revision.
ilya-biryukov added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/GlobalCompilationDatabase.cpp:90
+
+ Logger.log("Failed to find compilation database for " + Twine(File) + "\n");
+ return nu
Nebiroth updated this revision to Diff 117327.
Nebiroth added a comment.
Fixed changes that were lost while merging with head.
https://reviews.llvm.org/D37150
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase
ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.
Some changes seem to be lost while merging with head.
Comment at: clangd/GlobalCompilationDatabase.cpp:75
+ auto CachedIt = CompilationDatabases.find
Nebiroth updated this revision to Diff 117217.
Nebiroth added a comment.
Fixed missed comments and suggstions.
https://reviews.llvm.org/D37150
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase.h
clangd/tool
ilya-biryukov added inline comments.
Comment at: clangd/GlobalCompilationDatabase.cpp:82
// FIXME(ibiryukov): Invalidate cached compilation databases on changes
+return Result;
Why remove this `FIXME`?
Comment at: clangd/tool/ClangdM
Nebiroth added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:20
#include
+#include
malaperle wrote:
> William, did you perhaps miss this comment? I don't think unistd.h makes
> sense here.
I indeed missed it.
https://reviews.llvm.org/D37150
_
malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/tool/ClangdMain.cpp:20
#include
+#include
William, did you perhaps miss this comment? I don't think unistd.h make
Nebiroth updated this revision to Diff 116559.
Nebiroth added a comment.
Fixed inverted compile_commands check logic that made tests fail.
More readable command arg checks.
https://reviews.llvm.org/D37150
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCompilationD
ilya-biryukov added a comment.
Could you please run `clang-format` on every submission?
Comment at: clangd/GlobalCompilationDatabase.cpp:98
+ {
+CompileCommandsDir = "/";
+return tryLoadDatabaseFromPath(CompileCommandsDir.getValue());
Is this some
Nebiroth updated this revision to Diff 116198.
Nebiroth added a comment.
More consistent logging in clangdmain.
Restructured argument checking in ClangdMain
Fixed empty compile-commands-dir triggering error messages.
Fixed failing standard tests.
https://reviews.llvm.org/D37150
Files:
clangd/
ilya-biryukov added a comment.
All lit test for clangd are currently failing if I apply your change.
Generally, please run `make check-clang-tools` to make sure your changes do not
introduce any regressions.
The tests are failing because when `--compile-commands-dir` is not specified
clangd now
Nebiroth updated this revision to Diff 115462.
Nebiroth marked an inline comment as done.
Nebiroth added a comment.
Fixed a few comments.
Rebased on latest clangd version.
https://reviews.llvm.org/D37150
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCompilationDa
ilya-biryukov added inline comments.
Comment at: clangd/GlobalCompilationDatabase.cpp:75
+ if (CachedIt != CompilationDatabases.end())
+return (CachedIt->second.get());
+ auto CDB = tooling::CompilationDatabase::loadFromDirectory(File, Error);
Parentheses s
Nebiroth updated this revision to Diff 114654.
Nebiroth added a comment.
Simpilified a pointer return value.
https://reviews.llvm.org/D37150
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilation
klimek added inline comments.
Comment at: clangd/GlobalCompilationDatabase.cpp:98-100
+if (CDB)
+ return CDB;
+return nullptr;
Isn't that the same as "return CDB"?
https://reviews.llvm.org/D37150
___
c
Nebiroth updated this revision to Diff 114371.
Nebiroth marked 10 inline comments as done.
Nebiroth added a comment.
Ran clang-format on modified files.
More minor refactoring.
https://reviews.llvm.org/D37150
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.c
ilya-biryukov added a comment.
Could you also `clang-format` the code please?
Comment at: clangd/ClangdServer.cpp:150
bool RunSynchronously,
+ llvm::Optional CompileCommandsDir,
llvm::Optional Res
Nebiroth updated this revision to Diff 114199.
Nebiroth marked 7 inline comments as done.
Nebiroth added a comment.
Modified CompileCommandsDir to only look in the specified path if the value is
set.
Moved CompileCommandsDir field to DirectoryBasedGlobalCompilationDatabase class.
Minor refactorin
ilya-biryukov added a comment.
See my comments.
Could you also `clang-format` the code please?
Comment at: clangd/ClangdServer.cpp:150
bool RunSynchronously,
+ std::string CompileCommands,
llvm::O
Nebiroth updated this revision to Diff 113895.
Nebiroth added a comment.
Added function to avoid code reuse and extra nesting.
https://reviews.llvm.org/D37150
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/GlobalCompilat
krasimir added inline comments.
Comment at: clangd/GlobalCompilationDatabase.cpp:89
+ // if --compileCommands arg was invoked, check value and override default
path
+ std::size_t found = CompileCommands.find_first_of("/");
+ std::string TempString = CompileCommands;
-
ilya-biryukov added a comment.
BTW, if you're only looking for providing extra compilation flags for some
files, you could use an extension to LSP that is already in clangd source tree.
(See tests from https://reviews.llvm.org/D34947 for more details).
But your change is also useful, as it allow
ilya-biryukov added inline comments.
Comment at: clangd/GlobalCompilationDatabase.cpp:29
+
+static cl::opt CompileCommands("compileCommands",
+ cl::desc("Start with absolute path to
compile_commands.json"));
Please place this fl
Nebiroth created this revision.
Adds compileCommands command line argument to specify an absolute path directly
to the requested compile_commands.json for flags.
https://reviews.llvm.org/D37150
Files:
clangd/GlobalCompilationDatabase.cpp
Index: clangd/GlobalCompilationDatabase.cpp
=
31 matches
Mail list logo
