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 firstname.lastname@example.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits