DmitryPolukhin added a comment. @nridge I'm sorry, I pushed version without all tests. Now I added test case for response files, it seems that clangd hasn't had it before this patch.
================ Comment at: clang-tools-extra/clangd/CompileCommands.cpp:222 + tooling::addExpandedResponseFiles(Cmd, Command.Directory, Tokenizer, *FS); + tooling::addTargetAndModeForProgramName(Cmd, Cmd.front()); auto &OptTable = clang::driver::getDriverOptTable(); ---------------- nridge wrote: > nridge wrote: > > The target needs to be added **after** the call to `SystemIncludeExtractor` > > later in this function (this is what D138546 is trying to fix). The reason > > is that `SystemIncludeExtractor` includes any `--target` flag in the > > compiler driver being queried for system includes, which may be gcc, which > > does not support `--target`. > (I guess we could make that change separately in D138546, but if we're > changing the place where the `--target` is added in this patch, I figure we > might as well move it directly to the desired place.) I think there are order problems here: - we need `--driver-mode=cl` injected here to make check on line #229 work as expected - if we don't inject it driver mode here, command line edits won't see the change; see how I modified test clangd/unittests/CompileCommandsTests.cpp line #203, with D138546 edits were not applied properly to driver mode I'll double check how it works on Windows but I expect that moving it after SystemIncludeExtractor will break proper driver mode detection. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
