kadircet added a comment.
Hi @topisani, thanks for working on clangd!
Yes that bug fell through the cracks since the logs proposed in the comments
were not making use of `-query-driver` option. Do you mind adding some logs to
the bug report, without your patch so that we can clearly see its effect.
Initially we left it out, because it is always hairy to parse command line
options textually, and also kind of unclear why clang wouldn't pick it up
itself once you have `isysroot` or `sysroot` set, with extra logs we can be
sure there are
no bugs in clang/clangd side, and it is just about a custom logic in the driver
you are using.
We also need some tests, could you add some to
`test/system-include-extractor.test` ?
================
Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:230
Lang = Arg.drop_front(2).trim();
+ else if (Arg == "-nostdinc")
+ PreservedArgs.push_back(Arg);
----------------
we should also handle `-nobuiltininc`, `--no-standard-includes` and
`-nostdinc++`.
================
Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:232
+ PreservedArgs.push_back(Arg);
+ else if (Arg == "--sysroot" && I + 1 < E) {
+ PreservedArgs.push_back(Arg);
----------------
could you also handle `isysroot` ?
================
Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:260
else
- DriverToIncludesCache[Key] = SystemIncludes =
- extractSystemIncludes(Key.first, Key.second, QueryDriverRegex);
+ DriverToIncludesCache[Key] = SystemIncludes = extractSystemIncludes(
+ Key.first, Key.second, PreservedArgs, QueryDriverRegex);
----------------
let's pass the `Cmd->CommandLine` to `extractSystemIncludes` and perform the
preservation logic there, so that we only do it once.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73453/new/
https://reviews.llvm.org/D73453
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
