sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124970
Files:
clang-tools-extra/clangd/Compiler.cpp
clang/include/clang/Driver/Driver.h
clang/include/clang/Frontend/Utils.h
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
clang/unittests/Frontend/UtilsTest.cpp
Index: clang/unittests/Frontend/UtilsTest.cpp
===================================================================
--- clang/unittests/Frontend/UtilsTest.cpp
+++ 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 @@
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 clang
Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===================================================================
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -29,7 +29,7 @@
std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
ArrayRef<const char *> ArgList, IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS, bool ShouldRecoverOnErorrs,
- std::vector<std::string> *CC1Args) {
+ std::vector<std::string> *CC1Args, bool ProbePrecompiled) {
assert(!ArgList.empty());
if (!Diags.get()) {
// No diagnostics engine was provided, so create our own diagnostics object
@@ -51,6 +51,9 @@
// Don't check that inputs exist, they may have been remapped.
TheDriver.setCheckInputsExist(false);
+ // Don't probe on the filesystem for PCHes of -include'd headers - we may have
+ // a VFS, they may be misversioned, etc.
+ TheDriver.setProbePrecompiled(ProbePrecompiled);
std::unique_ptr<driver::Compilation> C(TheDriver.BuildCompilation(Args));
if (!C)
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1364,7 +1364,8 @@
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 @@
// 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;
}
}
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -198,7 +198,8 @@
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();
Index: clang/include/clang/Frontend/Utils.h
===================================================================
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -200,6 +200,10 @@
/// \param CC1Args - if non-null, will be populated with the args to cc1
/// expanded from \p Args. May be set even if nullptr is returned.
///
+/// \param ProbePrecompiled - allow the driver to probe the filesystem for
+/// PCH files to replace -include with.
+/// FIXME: ProbePrecompiled=true is a poor, historical default.
+///
/// \return A CompilerInvocation, or nullptr if none was built for the given
/// argument vector.
std::unique_ptr<CompilerInvocation> createInvocationFromCommandLine(
@@ -208,7 +212,7 @@
IntrusiveRefCntPtr<DiagnosticsEngine>(),
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr,
bool ShouldRecoverOnErrors = false,
- std::vector<std::string> *CC1Args = nullptr);
+ std::vector<std::string> *CC1Args = nullptr, bool ProbePrecompiled = true);
// Frontend timing utils
Index: clang/include/clang/Driver/Driver.h
===================================================================
--- clang/include/clang/Driver/Driver.h
+++ clang/include/clang/Driver/Driver.h
@@ -271,6 +271,9 @@
/// 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 @@
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; }
Index: clang-tools-extra/clangd/Compiler.cpp
===================================================================
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -96,7 +96,7 @@
CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false);
std::unique_ptr<CompilerInvocation> CI = createInvocationFromCommandLine(
ArgStrs, CommandLineDiagsEngine, std::move(VFS),
- /*ShouldRecoverOnErrors=*/true, CC1Args);
+ /*ShouldRecoverOnErrors=*/true, CC1Args, /*ProbePrecompiled=*/false);
if (!CI)
return nullptr;
// createInvocationFromCommandLine sets DisableFree.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
