[PATCH] D73453: Preserve -nostdinc and --sysroot when calling query driver

[Kadir Cetinkaya via Phabricator via cfe-commits]

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