ioeric marked 5 inline comments as done.
ioeric added inline comments.
================
Comment at: clangd/Headers.cpp:60
Argv.push_back(S.c_str());
IgnoringDiagConsumer IgnoreDiags;
auto CI = clang::createInvocationFromCommandLine(
----------------
ilya-biryukov wrote:
> Not related to this patch, but just noticed that we don't call
> `FS->setCurrentWorkingDirectory(CompileCommand.Directory)` here.
> This might break if `CompileCommand` has non-absolute paths.
This was done in ClangdServer, but we should probably do it here as you
suggested.
================
Comment at: clangd/index/FileIndex.cpp:36
+ CollectorOpts.IndexMainFiles = false;
+ CollectorOpts.CollectIncludePath = true;
+ const auto *Includes = CanonicalIncludesForSystemHeaders();
----------------
sammccall wrote:
> if this is always true now, we could remove the option?
Will do this in a followup patch. Added a FIXME
================
Comment at: clangd/index/FileIndex.cpp:37
+ CollectorOpts.CollectIncludePath = true;
+ const auto *Includes = CanonicalIncludesForSystemHeaders();
+ CollectorOpts.Includes = Includes;
----------------
sammccall wrote:
> This is not going to handle IWYU right?
> It seems like at this point the right thing to do is totally hide the
> CanonicalIncludes inside SymbolCollector, which would init it (to system
> headers), write IWYU mappings using the PP, and consume the mappings (when
> emitting symbols).
> Churn :(
You are right :(
I'll prepare a followup patch to handle this. Added a FIXME.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43462
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
