Clangd: Preserve target flags in system includes extractor

[cpsauer]

Summary

This preserves --target and -target flags in the system includes extractor, which is needed for cross-compiling to, e.g. Android, since the target flags can influence the default include paths.

Note that, like isysroot (which is handled incorrectly above and elsewhere), the target flag doesn't fit cleanly into the ArgsToPreserve abstraction and does indeed have a different number of - in its = and non = forms. (ref) There are plenty of bugs in this file, but this is an incremental improvement.

For more context, please see https://github.com/clangd/clangd/issues/1378

Thanks for all you do, wonderful LLVM folks :)

[sammccall]

My biggest concern here is propagating more information here without including it in the cache key.
Originally the computation was strictly using the cache key as input, but started using the command-line flags in D73453. @kadircet any extra context on when/why this is safe?

[cpsauer]

Sam, my read, too, is that the memoizing design isn't safe--also that the cache-key issue is preexisting.

I had the same reaction reading through this file after spotting problems in the log. That's what spawned https://github.com/clangd/clangd/issues/1378.

Any chance I could get you (and others) to quickly read through that issue if you haven't already? (The relevant section to this part: "If we think sysroots, targets, and the other flags enumerated effect the system includes, we'd better include them as part of the memoization key.")

There are sadly *lots* of problems in this file that leap out on a quick read. Nathan and I were thinking, though, that we'd should post this incremental fix for review rather than getting bogged down in trying to fix multiple things atomically.

[Also, if this does get approved, I'd love it if the approver could land it for me!]

[sammccall]

In D138546#3946046, @cpsauer wrote:

Sam, my read, too, is that the memoizing design isn't safe--also that the key bug is preexisting.
Nathan and I were thinking, though, that we'd should post this incremental fix for review rather than getting bogged down in trying to fix multiple things atomically.

Sure. At first glance the design looks like it's been changed in a way that's broken, but maybe there's some deeper reason that it's safe. That reason may or may not also apply to -target (e.g. if -target could plausibly differ across a project but other flags couldn't). I wanted to understand whether/why it's broken today before concluding it's safe to break it further. Probably @kadircet is the best person to make a call here.

I had the same reaction reading through this file after spotting problems in the log. That's what spawned https://github.com/clangd/clangd/issues/1378.

Any chance I could get you (and others) to quickly read through that issue if you haven't already? (The relevant section to this part: "If we think sysroots, targets, and the other flags enumerated effect the system includes, we'd better include them as part of the memoization key.") )

The discussion in this bug makes sense to me. I agree with the need for memoization, and the handling of -isysroot indeed looks dodgy and could probably be fixed.
Framework support sounds important for Mac, and we'd be happy to take a contribution from a Mac person who can explain the issues there.
Reusing clang's arg parser is tricky: it's a large change, it's often more complex than naive string-bashing, and it's also significantly slower.

Unfortunately at the moment I'm not really able to spend work time on improving these things, beyond reviewing patches.
(The system include extractor is mostly useful for things that don't build with stock clang, which at $employer is a minority).

[cpsauer]

Makes sense! Thanks for your time, Sam, and for being great, as always.

[cpsauer]

I suppose, if it ever might help make the case with said employer (Google, right?) the specific broken use case that's motivating this is Android (and also usage from Bazel/Blaze and by Google projects).

[nridge]

In D138546#3946144, @sammccall wrote:

In D138546#3946046, @cpsauer wrote:

Sam, my read, too, is that the memoizing design isn't safe--also that the key bug is preexisting.
Nathan and I were thinking, though, that we'd should post this incremental fix for review rather than getting bogged down in trying to fix multiple things atomically.

Sure. At first glance the design looks like it's been changed in a way that's broken, but maybe there's some deeper reason that it's safe. That reason may or may not also apply to -target (e.g. if -target could plausibly differ across a project but other flags couldn't). I wanted to understand whether/why it's broken today before concluding it's safe to break it further. Probably @kadircet is the best person to make a call here.

Given that a "project" is a fuzzy concept -- where e.g. someone in a cross-compilation scenario may choose to group host tools and target tools together in the same project -- surely -isysroot, -nostdinc etc. could also potentially differ across a project (but it's probably not common).

I think the simple answer here is that not including those flags in the key was an oversight, albeit a low-impact one.

I'd say taking this change as-is would be a net improvement (but we should also fix the oversight, now that we're aware of it, in a follow-up change).

My only suggestion regarding this patch would be to amend this test to check for the preservation of the target flag as well.

[kadircet]

Sorry for noticing this so late. Yes the initial reason we left sysroot and std/builtin-inc related flags were exactly that, in theory they could vary but in practice they were applied to all or none of the project.

Regarding this change itself, surely preserving target makes sense and similar to sysroot and others, i'd actually expect it to be the same across all files in practice. But they're more likely to change without a clangd restart (e.g. because user invokes build in a different configuration). So i think they should be part of the cache key nevertheless. My main question about this change is, you seem to be using a clang driver underneath. Why is clang driver we have in clangd not able to infer the same search paths etc? Is the clang provided in android ndk a custom clang? can you provide logs in the issue you've mentioned above, especially the compile flags retrieved from CDB and cc1 produced by it?

Regarding the mishandling of -isysroot<dir> vs -isysroot <dir> (they still both have a single -, but expecting an equals in between is a mistake apparently), surely we should fix that.

[cpsauer]

Sorry for being so slow this time myself, guys. Took me a while to dig myself out of a deep inbox hole after Thanksgiving (US holiday). Really appreciate you all and your friendly responsiveness, especially as I get up to speed here.

[cpsauer]

@nridge, I took a shot at amending the test. Thanks for the pointer! Please me know if that looks good to you!
The host-target cross-compile situation you hypothesize is exactly what caused me to notice this. Super common with Bazel.

[cpsauer]

@kadircet, you were asking about what's requiring the use of query-driver--I'll reply over in the issue (as you suggest), keeping everything in one place.

[nridge]

In D138546#3980350, @cpsauer wrote:

@nridge, I took a shot at amending the test. Thanks for the pointer! Please me know if that looks good to you!

The test changes look good, thanks.

I'm going to leave the approval to Kadir because his comment sounded like he would maybe prefer for https://github.com/clangd/clangd/issues/1403 (note, it has a patch at D139277) to be fixed before merging this.

[cpsauer]

Sweet. Thanks again, Nathan.

Guys, lmk how you'd like to go from here. If you approve; feel free to copy in/land as part of the other change if that would save time.
(I don't have commit access anyway.)

[ArcsinX]

It seems this patch can break GCC toolchains support

This patch expects that we never get target-related options in CommandLine passed to extractSystemIncludesAndTarget() for GCC toolchain. But seems this is not always true because of https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp#L253

In other words, with this patch we preserve --target=..., but this option can appear from https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp#L253 (but not from compile_commands.json). As the result of it, we can call GCC toolchain with --target=.. option at trying to query-driver and this call will fail (because GCC has no --target option)

[cpsauer]

Ooh interesting. Thanks for your careful look, @ArcsinX.

To check my understanding: Sounds like, more generally, commands get massaged into clang argument form before they're passed into the query-driver machinery, which can break break calls through to other compilers?

So far that hadn't affected things, but probably we should be relayering things to operate on raw commands for query driver?
@nridge, I know you'd fixed a bunch of problems by relayering in the past. Any thoughts on this one?

More generally folks more experienced in the codebase: Any thoughts on how we should do this?

[nridge]

In D138546#4017356, @ArcsinX wrote:

In other words, with this patch we preserve --target=..., but this option can appear from https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp#L253

Huh, I had no idea we had logic to add a --target flag there. (Nor that gcc does not recognize this option.)

@cpsauer just to double-check, in your case the --target flag is explicitly written in the compile_commands.json, and the idea is that in such case we want to pass it to the system include extractor (since it's safe to assume the compiler supports the flag in that case), but not in the case where we add it?

In D138546#4018119, @cpsauer wrote:

probably we should be relayering things to operate on raw commands for query driver?

Not quite raw commands: the fix for https://github.com/clangd/clangd/issues/1173 was to make sure the inferred language (e.g. -x c++) is added before query-driver, and the fix for https://github.com/clangd/clangd/issues/1089 was to make sure that edits from the config file are applied before query-driver (because Compiler can be overridden there).

But I think we could achieve the intended effect by removing the call to inferTargetAndDriverMode() from GlobalCompilationDatabase.cpp, and instead performing the equivalent logic in CommandMangler, after invoking the SystemIncludeExtractor?

Any thoughts on whether that would be reasonable / whether it would break anything else, are welcome.

[cpsauer]

@nridge, yep, confirming: For Android, --target is being added explicitly as part of the command and we'll need to pass through explicit (but not implicit) --target flags to the system includes extractor to get the correct paths.

[cpsauer]

@nridge, I took a shot at the change you suggested. Confirming that this what you were thinking of, removing inferTargetAndDriverMode from database load and replacing it with a call to the underlying addTargetAndModeForProgramName in the CommandMangler after the SystemIncludeExtractor is invoked?
Note that I also saw a parallel call to inferTargetAndDriverMode in JSONCompilationDatabasePlugin, which seemed like it might cause the same problem, so I eliminated the call in there, too. I then culled the dead code, since there weren't other call sites for the addTargetAndModeForProgramName wrappings. Could I ask you to sanity check that? I know that's a more general tooling library; I was having trouble determining whether other tooling needed the target inference or not.
Also, @ArcsinX, could I get your take? Do you think this would fix the layering issue you raised?

We're definitely stretching my (currently quite limited) understanding of how clangd's implementation fits together, so I think I'll need to ask for help/guidance if that looks wrong.

Thanks again, guys!
-CS

[cpsauer]

(If anyone knows what's going on with the (unrelated seeming?) testing timeouts please do say.)

[nridge]

In D138546#4053538, @cpsauer wrote:

@nridge, I took a shot at the change you suggested. Confirming that this what you were thinking of, removing inferTargetAndDriverMode from database load and replacing it with a call to the underlying addTargetAndModeForProgramName in the CommandMangler after the SystemIncludeExtractor is invoked?

Yup, that's what I had in mind. Thanks for writing that up!

Note that I also saw a parallel call to inferTargetAndDriverMode in JSONCompilationDatabasePlugin, which seemed like it might cause the same problem, so I eliminated the call in there, too.

Oh, huh, I didn't realize that JSONCompilationDatabasePlugin has this behaviour baked in.

I would err on the side of leaving that call site as is.

My thinking is as follows:

• The general purpose of the compilation database utilities in libTooling is to produce command lines that will be fed to the clang driver library.
• The clang driver does support --target, and in some cases having that flag present is important.
• For clangd's call site, we're not removing the logic to add the --target flag, we're just moving it from an ealier stage of the pipeline to a later stage of the pipeline, because our pipeline contains a specialized step (SystemIncludeExtractor) that may involve executing a gcc driver with (parts of the) the command.
• But for other tools built on libTooling that use JSONCompilationDatabasePlugin, this change would be removing the --target logic altogether, which could regress behaviour.

I then culled the dead code, since there weren't other call sites for the addTargetAndModeForProgramName wrappings.

Reagrdless of what we decide about JSONCompilationDatabasePlugin, we probably shouldn't remove clang::tooling::inferTargetAndDriverMode(), as that's a public libTooling API that may be used by external projects that use libTooling as a library.

[cpsauer]

Yay. Thanks, Nathan, for guiding, replying, and just generally being so kind, great, and thorough.

I'll back out the plugin part of the changes changes momentarily, then.
Just to confirm: Sounds like you're not concerned, then, that the JSONCompilationDatabasePlugin will get invoked and the results then passed to a SystemIncludeExtractor-enabled mangler?
(I'd seen some fallback calls into the plugin registry in GlobalCompilationDatabase, hence the original change, but, lacking broader context, I was having trouble tracing the expected control flow.)

[cpsauer]

Rebased

[nridge]

In D138546#4075681, @cpsauer wrote:

Sounds like you're not concerned, then, that the JSONCompilationDatabasePlugin will get invoked and the results then passed to a SystemIncludeExtractor-enabled mangler?
(I'd seen some fallback calls into the plugin registry in GlobalCompilationDatabase, hence the original change, but, lacking broader context, I was having trouble tracing the expected control flow.)

I did initially overlook the mentioned fallback codepath, thanks for catching that. However, it looks like it's only taken if we've already explicitly checked for compile_commands.json and didn't find one, so JsonCompilationDatabasePlugin won't find anything either (i.e. the fallback is for potential other plugins).

As for consumers of the plugin registry outside of clangd, we can assume they won't use SystemIncludeExtractor which is currently a clangd-internal component. (There has been an idea floated about upstreaming SystemIncludeExtractor from clangd to libTooling, but it remains to be determined how that would look at the API level, and we can make adjustments as necessary if/when that happens.)

Anyways, +1 from me on the TargetAndMode related changes.

[cpsauer]

Thanks, Nathan. Makes sense; sounds good.

@kadircet, I think this one is ready for you, whenever you want it. Lmk how you'd like to proceed.

[cpsauer]

@kadircet, friendly check in, since I got reminded of this: How would you like to proceed from here?

[cpsauer]

@kadircet, could I get your review on this one?

[kadircet]

Sorry I was waiting for D139277 to land, but it seems to be stuck so sent out D146941. Let's take a look at this again after that one lands

[nridge]

This can be closed, as the change has been made in https://github.com/llvm/llvm-project/pull/65824

clang-tools-extra/clangd/CompileCommands.cpp

 //===--- CompileCommands.cpp ----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "CompileCommands.h"
 #include "Config.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Debug.h"

 CommandMangler CommandMangler::forTests() { return CommandMangler(); }
 
 void CommandMangler::operator()(tooling::CompileCommand &Command,
                                 llvm::StringRef File) const {
   std::vector<std::string> &Cmd = Command.CommandLine;
   trace::Span S("AdjustCompileFlags");
   // Most of the modifications below assumes the Cmd starts with a driver name.
   // We might consider injecting a generic driver name like "cc" or "c++", but
-  // a Cmd missing the driver is probably rare enough in practice and errnous.
+  // a Cmd missing a driver is probably rare enough in practice and erroneous.
   if (Cmd.empty())
     return;
   auto &OptTable = clang::driver::getDriverOptTable();
   // OriginalArgs needs to outlive ArgList.
   llvm::SmallVector<const char *, 16> OriginalArgs;
   OriginalArgs.reserve(Cmd.size());
   for (const auto &S : Cmd)
     OriginalArgs.push_back(S.c_str());
void CommandMangler::operator()(tooling::CompileCommand &Command,
   for (auto &Edit : Config::current().CompileFlags.Edits)
     Edit(Cmd);
 
   // The system include extractor needs to run:
   //  - AFTER transferCompileCommand(), because the -x flag it adds may be
   //    necessary for the system include extractor to identify the file type
   //  - AFTER applying CompileFlags.Edits, because the name of the compiler
   //    that needs to be invoked may come from the CompileFlags->Compiler key
+  //  - BEFORE addTargetAndModeForProgramName(), because gcc doesn't support
+  //    the target flag that might be added.
   //  - BEFORE resolveDriver() because that can mess up the driver path,
   //    e.g. changing gcc to /path/to/clang/bin/gcc
   if (SystemIncludeExtractor) {
     SystemIncludeExtractor(Command, File);
   }
 
+  tooling::addTargetAndModeForProgramName(Cmd, Cmd.front());
+
   // Check whether the flag exists, either as -flag or -flag=*
   auto Has = [&](llvm::StringRef Flag) {
     for (llvm::StringRef Arg : Cmd) {
       if (Arg.consume_front(Flag) && (Arg.empty() || Arg[0] == '='))
         return true;
     }
     return false;
   };

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

parseJSON(PathRef Path, llvm::StringRef Data, std::string &Error) {
   if (auto CDB = tooling::JSONCompilationDatabase::loadFromBuffer(
           Data, Error, tooling::JSONCommandLineSyntax::AutoDetect)) {
     // FS used for expanding response files.
     // FIXME: ExpandResponseFilesDatabase appears not to provide the usual
     // thread-safety guarantees, as the access to FS is not locked!
     // For now, use the real FS, which is known to be threadsafe (if we don't
     // use/change working directory, which ExpandResponseFilesDatabase doesn't).
     auto FS = llvm::vfs::getRealFileSystem();
-    return tooling::inferTargetAndDriverMode(
-        tooling::inferMissingCompileCommands(
-            expandResponseFiles(std::move(CDB), std::move(FS))));
+    return tooling::inferMissingCompileCommands(
+        expandResponseFiles(std::move(CDB), std::move(FS)));
   }
   return nullptr;
 }
 static std::unique_ptr<tooling::CompilationDatabase>
 parseFixed(PathRef Path, llvm::StringRef Data, std::string &Error) {
   return tooling::FixedCompilationDatabase::loadFromBuffer(
       llvm::sys::path::parent_path(Path), Data, Error);
 }

clang-tools-extra/clangd/SystemIncludeExtractor.cpp

extractSystemIncludesAndTarget(llvm::SmallString<128> Driver,
 
   llvm::SmallVector<llvm::StringRef> Args = {Driver, "-E", "-x",
                                              Lang,   "-",  "-v"};
 
   // These flags will be preserved
   const llvm::StringRef FlagsToPreserve[] = {
       "-nostdinc", "--no-standard-includes", "-nostdinc++", "-nobuiltininc"};
   // Preserves these flags and their values, either as separate args or with an
-  // equalsbetween them
+  // equals between them
   const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"};
 
   for (size_t I = 0, E = CommandLine.size(); I < E; ++I) {
     llvm::StringRef Arg = CommandLine[I];
     if (llvm::is_contained(FlagsToPreserve, Arg)) {
       Args.push_back(Arg);
+    } else if (Arg.startswith("--target=")) {
+      Args.push_back(Arg);
+    } else if (I + 1 < E && Arg.equals("-target")) {
+      Args.push_back(CommandLine[I]);
+      Args.push_back(CommandLine[++I]);
     } else {
       const auto *Found =
           llvm::find_if(ArgsToPreserve, [&Arg](llvm::StringRef S) {
             return Arg.startswith(S);
           });
       if (Found == std::end(ArgsToPreserve))
         continue;
       Arg = Arg.drop_front(Found->size());

clang-tools-extra/clangd/test/system-include-extractor.test

 # RUN: echo '#!/bin/sh' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ "$0" = "%t.dir/bin/my_driver.sh" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'args="$*"' >> %t.dir/bin/my_driver.sh
 # Check that clangd preserves certain flags like `-nostdinc` from
 # original invocation in compile_commands.json.
 # RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"-target arm-linux-gnueabi"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"--target=arm-linux-gnueabi"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir2/ >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "End of search list.\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: chmod +x %t.dir/bin/my_driver.sh
 
 # Create header files my/dir/a.h and my/dir2/b.h
 # RUN: mkdir -p %t.dir/my/dir
 # RUN: touch %t.dir/my/dir/a.h
 # RUN: mkdir -p %t.dir/my/dir2
 # RUN: touch %t.dir/my/dir2/b.h
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot -target arm-linux-gnueabi --target=arm-linux-gnueabi", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
 # (with the extra slash in the front), so we add it here.
 # RUN: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' %t.test.1 > %t.test
 
 # Bless the mock driver we've just created so that clangd can execute it.
 # Note: include clangd's stderr in the FileCheck input with "2>&1" so that we

 # CHECK-NEXT:     "diagnostics": [],
 # CHECK-NEXT:     "uri": "file://{{.*}}/the-file.cpp",
 ---
 {"jsonrpc":"2.0","id":10000,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
 
 # Generate a different compile_commands.json which does not point to the mock driver
-# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot -target arm-linux-gnueabi --target=arm-linux-gnueabi", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # Generate a clangd config file which points to the mock driver instead
 # RUN: echo 'CompileFlags:' > %t.dir/.clangd
 # RUN: echo '  Compiler: my_driver.sh' >> %t.dir/.clangd
 
 # Run clangd a second time, to make sure it picks up the driver name from the config file
 # Note, we need to pass -enable-config because -lit-test otherwise disables it
 # RUN: clangd -lit-test -enable-config -query-driver="**.test,**.sh" < %t.test 2>&1 | FileCheck -strict-whitespace %t.test

clang/docs/tools/clang-formatted-files.txt

 clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
 clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp
 clang/lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
 clang/lib/Testing/CommandLineArgs.cpp
 clang/lib/Tooling/ArgumentsAdjusters.cpp
 clang/lib/Tooling/Execution.cpp
 clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
 clang/lib/Tooling/FixIt.cpp
-clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp
 clang/lib/Tooling/NodeIntrospection.cpp
 clang/lib/Tooling/StandaloneExecution.cpp
 clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
 clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
 clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
 clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
 clang/lib/Tooling/DumpTool/APIData.h
 clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h

clang/include/clang/Tooling/CompilationDatabase.h

 
 /// Returns a wrapped CompilationDatabase that defers to the provided one,
 /// but getCompileCommands() will infer commands for unknown files.
 /// The return value of getAllFiles() or getAllCompileCommands() is unchanged.
 /// See InterpolatingCompilationDatabase.cpp for details on heuristics.
 std::unique_ptr<CompilationDatabase>
     inferMissingCompileCommands(std::unique_ptr<CompilationDatabase>);
 
-/// Returns a wrapped CompilationDatabase that will add -target and -mode flags
-/// to commandline when they can be deduced from argv[0] of commandline returned
-/// by underlying database.
-std::unique_ptr<CompilationDatabase>
-inferTargetAndDriverMode(std::unique_ptr<CompilationDatabase> Base);
-
 /// Returns a wrapped CompilationDatabase that will expand all rsp(response)
 /// files on commandline returned by underlying database.
 std::unique_ptr<CompilationDatabase>
 expandResponseFiles(std::unique_ptr<CompilationDatabase> Base,
                     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
 
 } // namespace tooling
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLING_COMPILATIONDATABASE_H

clang/lib/Tooling/CMakeLists.txt

add_clang_library(clangTooling
   AllTUsExecution.cpp
   ArgumentsAdjusters.cpp
   CommonOptionsParser.cpp
   CompilationDatabase.cpp
   Execution.cpp
   ExpandResponseFilesCompilationDatabase.cpp
   FileMatchTrie.cpp
   FixIt.cpp
-  GuessTargetAndModeCompilationDatabase.cpp
   InterpolatingCompilationDatabase.cpp
   JSONCompilationDatabase.cpp
   Refactoring.cpp
   RefactoringCallbacks.cpp
   StandaloneExecution.cpp
   NodeIntrospection.cpp
   ${BINARY_INCLUDE_DIR}/NodeIntrospection.inc
   Tooling.cpp

clang/lib/Tooling/GuessTargetAndModeCompilationDatabase.cpp

-//===- GuessTargetAndModeCompilationDatabase.cpp --------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Tooling/Tooling.h"
-#include <memory>
-
-namespace clang {
-namespace tooling {
-
-namespace {
-class TargetAndModeAdderDatabase : public CompilationDatabase {
-public:
-  TargetAndModeAdderDatabase(std::unique_ptr<CompilationDatabase> Base)
-      : Base(std::move(Base)) {
-    assert(this->Base != nullptr);
-  }
-
-  std::vector<std::string> getAllFiles() const override {
-    return Base->getAllFiles();
-  }
-
-  std::vector<CompileCommand> getAllCompileCommands() const override {
-    return addTargetAndMode(Base->getAllCompileCommands());
-  }
-
-  std::vector<CompileCommand>
-  getCompileCommands(StringRef FilePath) const override {
-    return addTargetAndMode(Base->getCompileCommands(FilePath));
-  }
-
-private:
-  std::vector<CompileCommand>
-  addTargetAndMode(std::vector<CompileCommand> Cmds) const {
-    for (auto &Cmd : Cmds) {
-      if (Cmd.CommandLine.empty())
-        continue;
-      addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine.front());
-    }
-    return Cmds;
-  }
-  std::unique_ptr<CompilationDatabase> Base;
-};
-} // namespace
-
-std::unique_ptr<CompilationDatabase>
-inferTargetAndDriverMode(std::unique_ptr<CompilationDatabase> Base) {
-  return std::make_unique<TargetAndModeAdderDatabase>(std::move(Base));
-}
-
-} // namespace tooling
-} // namespace clang

clang/lib/Tooling/JSONCompilationDatabase.cpp

 // compile commands for files not present in the database.
 class JSONCompilationDatabasePlugin : public CompilationDatabasePlugin {
   std::unique_ptr<CompilationDatabase>
   loadFromDirectory(StringRef Directory, std::string &ErrorMessage) override {
     SmallString<1024> JSONDatabasePath(Directory);
     llvm::sys::path::append(JSONDatabasePath, "compile_commands.json");
     auto Base = JSONCompilationDatabase::loadFromFile(
         JSONDatabasePath, ErrorMessage, JSONCommandLineSyntax::AutoDetect);
-    return Base ? inferTargetAndDriverMode(
-                      inferMissingCompileCommands(expandResponseFiles(
-                          std::move(Base), llvm::vfs::getRealFileSystem())))
+    return Base ? inferMissingCompileCommands(expandResponseFiles(
+                      std::move(Base), llvm::vfs::getRealFileSystem()))
                 : nullptr;
   }
 };
 
 } // namespace
 
 // Register the JSONCompilationDatabasePlugin with the
 // CompilationDatabasePluginRegistry using this statically initialized variable.

clang/unittests/Tooling/CompilationDatabaseTest.cpp

TEST(CompileCommandTest, EqualityOperator) {
   EXPECT_TRUE(CCRef != CCTest);
 
   CCTest = CCRef;
   CCTest.Output = "bonjour.o";
   EXPECT_FALSE(CCRef == CCTest);
   EXPECT_TRUE(CCRef != CCTest);
 }
 
-class TargetAndModeTest : public MemDBTest {
-public:
-  TargetAndModeTest() { llvm::InitializeAllTargetInfos(); }
-
-protected:
-  // Look up the command from a relative path, and return it in string form.
-  std::string getCommand(llvm::StringRef F) {
-    auto Results = inferTargetAndDriverMode(std::make_unique<MemCDB>(Entries))
-                       ->getCompileCommands(path(F));
-    if (Results.empty())
-      return "none";
-    return llvm::join(Results[0].CommandLine, " ");
-  }
-};
-
-TEST_F(TargetAndModeTest, TargetAndMode) {
-  add("foo.cpp", "clang-cl", "");
-  add("bar.cpp", "clang++", "");
-
-  EXPECT_EQ(getCommand("foo.cpp"),
-            "clang-cl --driver-mode=cl foo.cpp -D foo.cpp");
-  EXPECT_EQ(getCommand("bar.cpp"),
-            "clang++ --driver-mode=g++ bar.cpp -D bar.cpp");
-}
-
 class ExpandResponseFilesTest : public MemDBTest {
 public:
   ExpandResponseFilesTest() : FS(new llvm::vfs::InMemoryFileSystem) {}
 
 protected:
   void addFile(StringRef File, StringRef Content) {
     ASSERT_TRUE(
         FS->addFile(File, 0, llvm::MemoryBuffer::getMemBufferCopy(Content)));

llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn

sources = [
     "AllTUsExecution.cpp",
     "ArgumentsAdjusters.cpp",
     "CommonOptionsParser.cpp",
     "CompilationDatabase.cpp",
     "Execution.cpp",
     "ExpandResponseFilesCompilationDatabase.cpp",
     "FileMatchTrie.cpp",
     "FixIt.cpp",
-    "GuessTargetAndModeCompilationDatabase.cpp",
     "InterpolatingCompilationDatabase.cpp",
     "JSONCompilationDatabase.cpp",
     "NodeIntrospection.cpp",
     "Refactoring.cpp",
     "RefactoringCallbacks.cpp",
     "StandaloneExecution.cpp",
     "Tooling.cpp",
   ]
 }