nridge added a comment. In D138546#4053538 <https://reviews.llvm.org/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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138546/new/ https://reviews.llvm.org/D138546 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
