From 6e8d6bc9ec8739ec22b73a23f740f171f452e234 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Wed, 5 Feb 2020 09:55:56 +0100 Subject: [PATCH] [clangd] Preserve -nostdinc and --sysroot when calling query driver Solves this issue: https://github.com/clangd/clangd/issues/157 This is my first contribution to an llvm project, so I hope I'm doing it right! Patch by @topisani (Tobias Pisani)! Reviewers: kadircet, klimek Differential Revision: https://reviews.llvm.org/D73811 --- clang-tools-extra/clangd/QueryDriverDatabase.cpp | 56 ++++++++++++++++------ .../clangd/test/system-include-extractor.test | 8 +++- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp b/clang-tools-extra/clangd/QueryDriverDatabase.cpp index e4f5c48..20eb4f8 100644 --- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp +++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp @@ -85,9 +85,10 @@ std::vector parseDriverOutput(llvm::StringRef Output) { return SystemIncludes; } -std::vector extractSystemIncludes(PathRef Driver, - llvm::StringRef Lang, - llvm::Regex &QueryDriverRegex) { +std::vector +extractSystemIncludes(PathRef Driver, llvm::StringRef Lang, + llvm::ArrayRef CommandLine, + llvm::Regex &QueryDriverRegex) { trace::Span Tracer("Extract system includes"); SPAN_ATTACH(Tracer, "driver", Driver); SPAN_ATTACH(Tracer, "lang", Lang); @@ -120,14 +121,43 @@ std::vector extractSystemIncludes(PathRef Driver, llvm::Optional Redirects[] = { {""}, {""}, llvm::StringRef(StdErrPath)}; - // Should we also preserve flags like "-sysroot", "-nostdinc" ? - const llvm::StringRef Args[] = {Driver, "-E", "-x", Lang, "-", "-v"}; + llvm::SmallVector 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 + const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"}; + + for (size_t I = 0, E = CommandLine.size(); I < E; ++I) { + llvm::StringRef Arg = CommandLine[I]; + if (llvm::any_of(FlagsToPreserve, + [&Arg](llvm::StringRef S) { return S == Arg; })) { + Args.push_back(Arg); + } else { + const auto *Found = + llvm::find_if(ArgsToPreserve, [&Arg](llvm::StringRef S) { + return Arg.startswith(S); + }); + if (Found == std::end(ArgsToPreserve)) + continue; + Arg.consume_front(*Found); + if (Arg.empty() && I + 1 < E) { + Args.push_back(CommandLine[I]); + Args.push_back(CommandLine[++I]); + } else if (Arg.startswith("=")) { + Args.push_back(CommandLine[I]); + } + } + } if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None, Redirects)) { elog("System include extraction: driver execution failed with return code: " - "{0}", - llvm::to_string(RC)); + "{0}. Args: ['{1}']", + llvm::to_string(RC), llvm::join(Args, "', '")); return {}; } @@ -237,20 +267,18 @@ public: llvm::SmallString<128> Driver(Cmd->CommandLine.front()); llvm::sys::fs::make_absolute(Cmd->Directory, Driver); - auto Key = std::make_pair(Driver.str(), Lang); + auto Key = std::make_pair(Driver.str().str(), Lang.str()); std::vector SystemIncludes; { std::lock_guard Lock(Mu); - auto It = DriverToIncludesCache.find( - {std::string(Key.first), std::string(Key.second)}); + auto It = DriverToIncludesCache.find(Key); if (It != DriverToIncludesCache.end()) SystemIncludes = It->second; else - DriverToIncludesCache[{std::string(Key.first), - std::string(Key.second)}] = SystemIncludes = - extractSystemIncludes(Key.first, Key.second, QueryDriverRegex); + DriverToIncludesCache[Key] = SystemIncludes = extractSystemIncludes( + Key.first, Key.second, Cmd->CommandLine, QueryDriverRegex); } return addSystemIncludes(*Cmd, SystemIncludes); @@ -280,7 +308,7 @@ getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs, if (QueryDriverGlobs.empty()) return Base; return std::make_unique(QueryDriverGlobs, - std::move(Base)); + std::move(Base)); } } // namespace clangd diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test b/clang-tools-extra/clangd/test/system-include-extractor.test index b61ad7a..b1551d8 100644 --- a/clang-tools-extra/clangd/test/system-include-extractor.test +++ b/clang-tools-extra/clangd/test/system-include-extractor.test @@ -5,8 +5,12 @@ # Generate a mock-driver that will print %temp_dir%/my/dir and # %temp_dir%/my/dir2 as include search paths. -# RUN: echo '#!/bin/bash' >> %t.dir/my_driver.sh +# RUN: echo '#!/bin/sh' >> %t.dir/my_driver.sh # RUN: echo '[ "$0" = "%t.dir/my_driver.sh" ] || exit' >> %t.dir/my_driver.sh +# RUN: echo 'args="$*"' >> %t.dir/my_driver.sh +# RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> %t.dir/my_driver.sh +# RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> %t.dir/my_driver.sh +# RUN: echo 'echo " $* " | grep " --sysroot /my/sysroot/path " || exit' >> %t.dir/my_driver.sh # RUN: echo 'echo line to ignore >&2' >> %t.dir/my_driver.sh # RUN: echo 'echo -e "#include <...> search starts here:\r" >&2' >> %t.dir/my_driver.sh # RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/my_driver.sh @@ -22,7 +26,7 @@ # 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": "%/t.dir/my_driver.sh the-file.cpp", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json +# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "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:/..." -- 2.7.4