ilya-biryukov added inline comments.
================
Comment at: clangd/GlobalCompilationDatabase.cpp:20
+ const SmallVectorImpl<StringRef> &ExtraFlags) {
+ assert(Command && !Command->CommandLine.empty());
+ if (!Command || ExtraFlags.empty())
----------------
Maybe pass `Command` by reference to avoid checking for null?
Is there a guideline to use pointers to make things more explicit that I've
missed?
================
Comment at: clangd/GlobalCompilationDatabase.cpp:21
+ assert(Command && !Command->CommandLine.empty());
+ if (!Command || ExtraFlags.empty())
+ return;
----------------
There's an assert that `Command` is always non-null, so `!Command` is always
`false`.
Maybe remove this check?
================
Comment at: clangd/GlobalCompilationDatabase.cpp:23
+ return;
+ if (Command->CommandLine.empty())
+ Command->CommandLine.push_back("clang");
----------------
There's an assert that checks that `CommandLine` is never empty.
Maybe remove this if statement?
================
Comment at: clangd/GlobalCompilationDatabase.cpp:27
+ ++It;
+ Command->CommandLine.insert(It, ExtraFlags.begin(), ExtraFlags.end());
+}
----------------
This way extra flags won't override flags from compilation database, we should
put `ExtraFlags` right before the input files.
clangd will break if it receives CommandLine with more than one input file, so
it should be safe to assume that the last arg is the name of the input file,
i.e. always add `ExtraFlags` right before the last arg.
================
Comment at: clangd/GlobalCompilationDatabase.cpp:38
+ if (Commands.empty()) {
+ std::vector<std::string> CommandLine{"clang", "-fsyntax-only", File.str()};
+ Commands.push_back(tooling::CompileCommand(
----------------
Maybe extract a small helper function to get default `CompileCommand` and use
it both in `ClangdUnitStore` and here?
Just in case we'll ever want to change the defaults at some point.
================
Comment at: clangd/GlobalCompilationDatabase.cpp:47
+ SmallVector<StringRef, 4> UserFlags;
+ StringRef(It->second).split(UserFlags, " ");
+ // Append the user-specified flags to the compile commands.
----------------
This will break for options, containing spaces (e.g. `-I"/home/user/my dir with
spaces"`).
There should a function parsing command-line args somewhere in LLVM? Can we
avoid splitting command-line args altogether (see next comment)?
================
Comment at: clangd/GlobalCompilationDatabase.cpp:58
+ PathRef File, llvm::StringRef ExtraFlags) {
+ ExtraFlagsForFile[File] = ExtraFlags;
}
----------------
Maybe store splitted command-line args in `ExtraFlagsForFile`?
`ycm_extra_conf.py` seems to pass splited command-line arguments, maybe change
our extension to receive a list of arguments to avoid doing command-line arg
splitting?
https://reviews.llvm.org/D34947
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
