From 04b419048955fc33718ba97e79f3558b6a27830e Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Thu, 5 May 2022 01:15:28 +0200 Subject: [PATCH] [Driver] Make "upgrade" of -include to include-pch optional; disable in clangd If clang is passed "-include foo.h", it will rewrite to "-include-pch foo.h.pch" before passing it to cc1, if foo.h.pch exists. Existence is checked, but validity is not. This is probably a reasonable assumption for the compiler itself, but not for clang-based tools where the actual compiler may be a different version of clang, or even GCC. In the end, we lose our -include, we gain a -include-pch that can't be used, and the file often fails to parse. I would like to turn this off for all non-clang invocations (i.e. createInvocationFromCommandLine), but we have explicit tests of this behavior for libclang and I can't work out the implications of changing it. Instead this patch: - makes it optional in the driver, default on (no change) - makes it optional in createInvocationFromCommandLine, default on (no change) - changes driver to do IO through the VFS so it can be tested - tests the option - turns the option off in clangd where the problem was reported Subsequent patches should make libclang opt in explicitly and flip the default for all other tools. It's probably also time to extract an options struct for createInvocationFromCommandLine. Fixes https://github.com/clangd/clangd/issues/856 Fixes https://github.com/clangd/vscode-clangd/issues/324 Differential Revision: https://reviews.llvm.org/D124970 --- clang-tools-extra/clangd/Compiler.cpp | 1 + clang/include/clang/Driver/Driver.h | 6 +++++ clang/include/clang/Frontend/Utils.h | 7 +++++- clang/lib/Driver/Driver.cpp | 3 ++- clang/lib/Driver/ToolChains/Clang.cpp | 7 +++--- .../Frontend/CreateInvocationFromCommandLine.cpp | 7 +++--- clang/unittests/Frontend/UtilsTest.cpp | 29 ++++++++++++++++++++++ 7 files changed, 52 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp index a762b31..2cf2eb8 100644 --- a/clang-tools-extra/clangd/Compiler.cpp +++ b/clang-tools-extra/clangd/Compiler.cpp @@ -97,6 +97,7 @@ buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D, CIOpts.RecoverOnError = true; CIOpts.Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false); + CIOpts.ProbePrecompiled = false; std::unique_ptr CI = createInvocation(ArgStrs, CIOpts); if (!CI) return nullptr; diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index f68f281..eb739ef 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -271,6 +271,9 @@ private: /// Whether to check that input files exist when constructing compilation /// jobs. unsigned CheckInputsExist : 1; + /// Whether to probe for PCH files on disk, in order to upgrade + /// -include foo.h to -include-pch foo.h.pch. + unsigned ProbePrecompiled : 1; public: /// Force clang to emit reproducer for driver invocation. This is enabled @@ -357,6 +360,9 @@ public: void setCheckInputsExist(bool Value) { CheckInputsExist = Value; } + bool getProbePrecompiled() const { return ProbePrecompiled; } + void setProbePrecompiled(bool Value) { ProbePrecompiled = Value; } + void setTargetAndMode(const ParsedClangName &TM) { ClangNameParts = TM; } const std::string &getTitle() { return DriverTitle; } diff --git a/clang/include/clang/Frontend/Utils.h b/clang/include/clang/Frontend/Utils.h index 14116ee..47f795b 100644 --- a/clang/include/clang/Frontend/Utils.h +++ b/clang/include/clang/Frontend/Utils.h @@ -202,6 +202,11 @@ struct CreateInvocationOptions { /// if any errors were encountered. /// By default, always return null on errors. bool RecoverOnError = false; + /// Allow the driver to probe the filesystem for PCH files. + /// This is used to replace -include with -include-pch in the cc1 args. + /// FIXME: ProbePrecompiled=true is a poor, historical default. + /// It misbehaves if the PCH file is from GCC, has the wrong version, etc. + bool ProbePrecompiled = true; /// If set, the target is populated with the cc1 args produced by the driver. /// This may be populated even if createInvocation returns nullptr. std::vector *CC1Args = nullptr; @@ -236,7 +241,7 @@ std::unique_ptr createInvocationFromCommandLine( IntrusiveRefCntPtr(), IntrusiveRefCntPtr VFS = nullptr, bool ShouldRecoverOnErrors = false, - std::vector *CC1Args = nullptr); + std::vector *CC1Args = nullptr, bool ProbePrecompiled = true); } // namespace clang diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 068ab89..8f71634 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -198,7 +198,8 @@ Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple, CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false), CCGenDiagnostics(false), CCPrintProcessStats(false), TargetTriple(TargetTriple), Saver(Alloc), CheckInputsExist(true), - GenReproducer(false), SuppressMissingInputWarning(false) { + ProbePrecompiled(true), GenReproducer(false), + SuppressMissingInputWarning(false) { // Provide a sane fallback if no VFS is specified. if (!this->VFS) this->VFS = llvm::vfs::getRealFileSystem(); diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 388a292..ebe51bc 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -1364,7 +1364,8 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA, bool RenderedImplicitInclude = false; for (const Arg *A : Args.filtered(options::OPT_clang_i_Group)) { - if (A->getOption().matches(options::OPT_include)) { + if (A->getOption().matches(options::OPT_include) && + D.getProbePrecompiled()) { // Handling of gcc-style gch precompiled headers. bool IsFirstImplicitInclude = !RenderedImplicitInclude; RenderedImplicitInclude = true; @@ -1375,12 +1376,12 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA, // so that replace_extension does the right thing. P += ".dummy"; llvm::sys::path::replace_extension(P, "pch"); - if (llvm::sys::fs::exists(P)) + if (D.getVFS().exists(P)) FoundPCH = true; if (!FoundPCH) { llvm::sys::path::replace_extension(P, "gch"); - if (llvm::sys::fs::exists(P)) { + if (D.getVFS().exists(P)) { FoundPCH = true; } } diff --git a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp index dee7a91..2a98aab 100644 --- a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp +++ b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp @@ -48,6 +48,7 @@ clang::createInvocation(ArrayRef ArgList, // Don't check that inputs exist, they may have been remapped. TheDriver.setCheckInputsExist(false); + TheDriver.setProbePrecompiled(Opts.ProbePrecompiled); std::unique_ptr C(TheDriver.BuildCompilation(Args)); if (!C) @@ -107,8 +108,8 @@ clang::createInvocation(ArrayRef ArgList, std::unique_ptr clang::createInvocationFromCommandLine( ArrayRef Args, IntrusiveRefCntPtr Diags, IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErrors, - std::vector *CC1Args) { + std::vector *CC1Args, bool ProbePrecompiled) { return createInvocation( - Args, - CreateInvocationOptions{Diags, VFS, ShouldRecoverOnErrors, CC1Args}); + Args, CreateInvocationOptions{Diags, VFS, ShouldRecoverOnErrors, + ProbePrecompiled, CC1Args}); } diff --git a/clang/unittests/Frontend/UtilsTest.cpp b/clang/unittests/Frontend/UtilsTest.cpp index 61686c8..cb2b6e0 100644 --- a/clang/unittests/Frontend/UtilsTest.cpp +++ b/clang/unittests/Frontend/UtilsTest.cpp @@ -11,12 +11,15 @@ #include "clang/Basic/TargetOptions.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" +#include "clang/Lex/PreprocessorOptions.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/Support/VirtualFileSystem.h" #include "gmock/gmock.h" #include "gtest/gtest.h" namespace clang { namespace { +using testing::ElementsAre; TEST(BuildCompilerInvocationTest, RecoverMultipleJobs) { // This generates multiple jobs and we recover by using the first. @@ -33,5 +36,31 @@ TEST(BuildCompilerInvocationTest, RecoverMultipleJobs) { EXPECT_THAT(CI->TargetOpts->Triple, testing::StartsWith("i386-")); } +// buildInvocationFromCommandLine should not translate -include to -include-pch, +// even if the PCH file exists. +TEST(BuildCompilerInvocationTest, ProbePrecompiled) { + std::vector Args = {"clang", "-include", "foo.h", "foo.cpp"}; + auto FS = llvm::makeIntrusiveRefCnt(); + FS->addFile("foo.h", 0, llvm::MemoryBuffer::getMemBuffer("")); + FS->addFile("foo.h.pch", 0, llvm::MemoryBuffer::getMemBuffer("")); + + clang::IgnoringDiagConsumer D; + llvm::IntrusiveRefCntPtr CommandLineDiagsEngine = + clang::CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, + false); + // Default: ProbePrecompiled is true. + std::unique_ptr CI = createInvocationFromCommandLine( + Args, CommandLineDiagsEngine, FS, false, nullptr); + ASSERT_TRUE(CI); + EXPECT_THAT(CI->getPreprocessorOpts().Includes, ElementsAre()); + EXPECT_EQ(CI->getPreprocessorOpts().ImplicitPCHInclude, "foo.h.pch"); + + CI = createInvocationFromCommandLine(Args, CommandLineDiagsEngine, FS, false, + nullptr, /*ProbePrecompiled=*/false); + ASSERT_TRUE(CI); + EXPECT_THAT(CI->getPreprocessorOpts().Includes, ElementsAre("foo.h")); + EXPECT_EQ(CI->getPreprocessorOpts().ImplicitPCHInclude, ""); +} + } // namespace } // namespace clang -- 2.7.4