[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
This commit is contained in:
Sam McCall 2022-05-05 01:15:28 +02:00
parent 042a7a5f0d
commit 04b4190489
7 changed files with 52 additions and 8 deletions

View File

@ -97,6 +97,7 @@ buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D,
CIOpts.RecoverOnError = true; CIOpts.RecoverOnError = true;
CIOpts.Diags = CIOpts.Diags =
CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false); CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false);
CIOpts.ProbePrecompiled = false;
std::unique_ptr<CompilerInvocation> CI = createInvocation(ArgStrs, CIOpts); std::unique_ptr<CompilerInvocation> CI = createInvocation(ArgStrs, CIOpts);
if (!CI) if (!CI)
return nullptr; return nullptr;

View File

@ -271,6 +271,9 @@ private:
/// Whether to check that input files exist when constructing compilation /// Whether to check that input files exist when constructing compilation
/// jobs. /// jobs.
unsigned CheckInputsExist : 1; 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: public:
/// Force clang to emit reproducer for driver invocation. This is enabled /// Force clang to emit reproducer for driver invocation. This is enabled
@ -357,6 +360,9 @@ public:
void setCheckInputsExist(bool Value) { CheckInputsExist = Value; } void setCheckInputsExist(bool Value) { CheckInputsExist = Value; }
bool getProbePrecompiled() const { return ProbePrecompiled; }
void setProbePrecompiled(bool Value) { ProbePrecompiled = Value; }
void setTargetAndMode(const ParsedClangName &TM) { ClangNameParts = TM; } void setTargetAndMode(const ParsedClangName &TM) { ClangNameParts = TM; }
const std::string &getTitle() { return DriverTitle; } const std::string &getTitle() { return DriverTitle; }

View File

@ -202,6 +202,11 @@ struct CreateInvocationOptions {
/// if any errors were encountered. /// if any errors were encountered.
/// By default, always return null on errors. /// By default, always return null on errors.
bool RecoverOnError = false; 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. /// If set, the target is populated with the cc1 args produced by the driver.
/// This may be populated even if createInvocation returns nullptr. /// This may be populated even if createInvocation returns nullptr.
std::vector<std::string> *CC1Args = nullptr; std::vector<std::string> *CC1Args = nullptr;
@ -236,7 +241,7 @@ std::unique_ptr<CompilerInvocation> createInvocationFromCommandLine(
IntrusiveRefCntPtr<DiagnosticsEngine>(), IntrusiveRefCntPtr<DiagnosticsEngine>(),
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr, IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr,
bool ShouldRecoverOnErrors = false, bool ShouldRecoverOnErrors = false,
std::vector<std::string> *CC1Args = nullptr); std::vector<std::string> *CC1Args = nullptr, bool ProbePrecompiled = true);
} // namespace clang } // namespace clang

View File

@ -198,7 +198,8 @@ Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false), CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
CCGenDiagnostics(false), CCPrintProcessStats(false), CCGenDiagnostics(false), CCPrintProcessStats(false),
TargetTriple(TargetTriple), Saver(Alloc), CheckInputsExist(true), 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. // Provide a sane fallback if no VFS is specified.
if (!this->VFS) if (!this->VFS)
this->VFS = llvm::vfs::getRealFileSystem(); this->VFS = llvm::vfs::getRealFileSystem();

View File

@ -1364,7 +1364,8 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
bool RenderedImplicitInclude = false; bool RenderedImplicitInclude = false;
for (const Arg *A : Args.filtered(options::OPT_clang_i_Group)) { 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. // Handling of gcc-style gch precompiled headers.
bool IsFirstImplicitInclude = !RenderedImplicitInclude; bool IsFirstImplicitInclude = !RenderedImplicitInclude;
RenderedImplicitInclude = true; RenderedImplicitInclude = true;
@ -1375,12 +1376,12 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
// so that replace_extension does the right thing. // so that replace_extension does the right thing.
P += ".dummy"; P += ".dummy";
llvm::sys::path::replace_extension(P, "pch"); llvm::sys::path::replace_extension(P, "pch");
if (llvm::sys::fs::exists(P)) if (D.getVFS().exists(P))
FoundPCH = true; FoundPCH = true;
if (!FoundPCH) { if (!FoundPCH) {
llvm::sys::path::replace_extension(P, "gch"); llvm::sys::path::replace_extension(P, "gch");
if (llvm::sys::fs::exists(P)) { if (D.getVFS().exists(P)) {
FoundPCH = true; FoundPCH = true;
} }
} }

View File

@ -48,6 +48,7 @@ clang::createInvocation(ArrayRef<const char *> ArgList,
// Don't check that inputs exist, they may have been remapped. // Don't check that inputs exist, they may have been remapped.
TheDriver.setCheckInputsExist(false); TheDriver.setCheckInputsExist(false);
TheDriver.setProbePrecompiled(Opts.ProbePrecompiled);
std::unique_ptr<driver::Compilation> C(TheDriver.BuildCompilation(Args)); std::unique_ptr<driver::Compilation> C(TheDriver.BuildCompilation(Args));
if (!C) if (!C)
@ -107,8 +108,8 @@ clang::createInvocation(ArrayRef<const char *> ArgList,
std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine( std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
ArrayRef<const char *> Args, IntrusiveRefCntPtr<DiagnosticsEngine> Diags, ArrayRef<const char *> Args, IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS, bool ShouldRecoverOnErrors, IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS, bool ShouldRecoverOnErrors,
std::vector<std::string> *CC1Args) { std::vector<std::string> *CC1Args, bool ProbePrecompiled) {
return createInvocation( return createInvocation(
Args, Args, CreateInvocationOptions{Diags, VFS, ShouldRecoverOnErrors,
CreateInvocationOptions{Diags, VFS, ShouldRecoverOnErrors, CC1Args}); ProbePrecompiled, CC1Args});
} }

View File

@ -11,12 +11,15 @@
#include "clang/Basic/TargetOptions.h" #include "clang/Basic/TargetOptions.h"
#include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/CompilerInvocation.h"
#include "clang/Lex/PreprocessorOptions.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/VirtualFileSystem.h"
#include "gmock/gmock.h" #include "gmock/gmock.h"
#include "gtest/gtest.h" #include "gtest/gtest.h"
namespace clang { namespace clang {
namespace { namespace {
using testing::ElementsAre;
TEST(BuildCompilerInvocationTest, RecoverMultipleJobs) { TEST(BuildCompilerInvocationTest, RecoverMultipleJobs) {
// This generates multiple jobs and we recover by using the first. // 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-")); 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<const char *> Args = {"clang", "-include", "foo.h", "foo.cpp"};
auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
FS->addFile("foo.h", 0, llvm::MemoryBuffer::getMemBuffer(""));
FS->addFile("foo.h.pch", 0, llvm::MemoryBuffer::getMemBuffer(""));
clang::IgnoringDiagConsumer D;
llvm::IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
clang::CompilerInstance::createDiagnostics(new DiagnosticOptions, &D,
false);
// Default: ProbePrecompiled is true.
std::unique_ptr<CompilerInvocation> 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
} // namespace clang } // namespace clang