DmitryPolukhin updated this revision to Diff 505030.
DmitryPolukhin added a comment.
Update test to use any available target
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143436/new/
https://reviews.llvm.org/D143436
Files:
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
clang/include/clang/Testing/CommandLineArgs.h
clang/include/clang/Tooling/Tooling.h
clang/lib/Testing/CommandLineArgs.cpp
clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
clang/lib/Tooling/Tooling.cpp
clang/unittests/Tooling/ToolingTest.cpp
Index: clang/unittests/Tooling/ToolingTest.cpp
===================================================================
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -17,11 +17,11 @@
#include "clang/Frontend/FrontendAction.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Testing/CommandLineArgs.h"
#include "clang/Tooling/ArgumentsAdjusters.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/MC/TargetRegistry.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/TargetSelect.h"
#include "llvm/TargetParser/Host.h"
@@ -871,28 +871,10 @@
EXPECT_FALSE(HasFlag("-random-plugin"));
}
-namespace {
-/// Find a target name such that looking for it in TargetRegistry by that name
-/// returns the same target. We expect that there is at least one target
-/// configured with this property.
-std::string getAnyTarget() {
+TEST(addTargetAndModeForProgramName, AddsTargetAndMode) {
llvm::InitializeAllTargets();
- for (const auto &Target : llvm::TargetRegistry::targets()) {
- std::string Error;
- StringRef TargetName(Target.getName());
- if (TargetName == "x86-64")
- TargetName = "x86_64";
- if (llvm::TargetRegistry::lookupTarget(std::string(TargetName), Error) ==
- &Target) {
- return std::string(TargetName);
- }
- }
- return "";
-}
-}
-TEST(addTargetAndModeForProgramName, AddsTargetAndMode) {
- std::string Target = getAnyTarget();
+ std::string Target = getAnyTargetForTesting();
ASSERT_FALSE(Target.empty());
std::vector<std::string> Args = {"clang", "-foo"};
@@ -905,7 +887,8 @@
}
TEST(addTargetAndModeForProgramName, PathIgnored) {
- std::string Target = getAnyTarget();
+ llvm::InitializeAllTargets();
+ std::string Target = getAnyTargetForTesting();
ASSERT_FALSE(Target.empty());
SmallString<32> ToolPath;
@@ -919,7 +902,8 @@
}
TEST(addTargetAndModeForProgramName, IgnoresExistingTarget) {
- std::string Target = getAnyTarget();
+ llvm::InitializeAllTargets();
+ std::string Target = getAnyTargetForTesting();
ASSERT_FALSE(Target.empty());
std::vector<std::string> Args = {"clang", "-foo", "-target", "something"};
@@ -936,7 +920,8 @@
}
TEST(addTargetAndModeForProgramName, IgnoresExistingMode) {
- std::string Target = getAnyTarget();
+ llvm::InitializeAllTargets();
+ std::string Target = getAnyTargetForTesting();
ASSERT_FALSE(Target.empty());
std::vector<std::string> Args = {"clang", "-foo", "--driver-mode=abc"};
Index: clang/lib/Tooling/Tooling.cpp
===================================================================
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -43,6 +43,7 @@
#include "llvm/Option/OptTable.h"
#include "llvm/Option/Option.h"
#include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/FileSystem.h"
@@ -299,6 +300,31 @@
}
}
+void addExpandedResponseFiles(std::vector<std::string> &CommandLine,
+ llvm::StringRef WorkingDir,
+ llvm::cl::TokenizerCallback Tokenizer,
+ llvm::vfs::FileSystem &FS) {
+ bool SeenRSPFile = false;
+ llvm::SmallVector<const char *, 20> Argv;
+ Argv.reserve(CommandLine.size());
+ for (auto &Arg : CommandLine) {
+ Argv.push_back(Arg.c_str());
+ if (!Arg.empty())
+ SeenRSPFile |= Arg.front() == '@';
+ }
+ if (!SeenRSPFile)
+ return;
+ llvm::BumpPtrAllocator Alloc;
+ llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
+ llvm::Error Err =
+ ECtx.setVFS(&FS).setCurrentDir(WorkingDir).expandResponseFiles(Argv);
+ if (Err)
+ llvm::errs() << Err;
+ // Don't assign directly, Argv aliases CommandLine.
+ std::vector<std::string> ExpandedArgv(Argv.begin(), Argv.end());
+ CommandLine = std::move(ExpandedArgv);
+}
+
} // namespace tooling
} // namespace clang
@@ -684,7 +710,7 @@
if (!Invocation.run())
return nullptr;
-
+
assert(ASTs.size() == 1);
return std::move(ASTs[0]);
}
Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/ConvertUTF.h"
@@ -48,28 +49,9 @@
private:
std::vector<CompileCommand> expand(std::vector<CompileCommand> Cmds) const {
- for (auto &Cmd : Cmds) {
- bool SeenRSPFile = false;
- llvm::SmallVector<const char *, 20> Argv;
- Argv.reserve(Cmd.CommandLine.size());
- for (auto &Arg : Cmd.CommandLine) {
- Argv.push_back(Arg.c_str());
- if (!Arg.empty())
- SeenRSPFile |= Arg.front() == '@';
- }
- if (!SeenRSPFile)
- continue;
- llvm::BumpPtrAllocator Alloc;
- llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
- llvm::Error Err = ECtx.setVFS(FS.get())
- .setCurrentDir(Cmd.Directory)
- .expandResponseFiles(Argv);
- if (Err)
- llvm::errs() << Err;
- // Don't assign directly, Argv aliases CommandLine.
- std::vector<std::string> ExpandedArgv(Argv.begin(), Argv.end());
- Cmd.CommandLine = std::move(ExpandedArgv);
- }
+ for (auto &Cmd : Cmds)
+ tooling::addExpandedResponseFiles(Cmd.CommandLine, Cmd.Directory,
+ Tokenizer, *FS);
return Cmds;
}
Index: clang/lib/Testing/CommandLineArgs.cpp
===================================================================
--- clang/lib/Testing/CommandLineArgs.cpp
+++ clang/lib/Testing/CommandLineArgs.cpp
@@ -7,7 +7,9 @@
//===----------------------------------------------------------------------===//
#include "clang/Testing/CommandLineArgs.h"
+#include "llvm/MC/TargetRegistry.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/TargetSelect.h"
namespace clang {
@@ -109,4 +111,18 @@
llvm_unreachable("Unhandled TestLanguage enum");
}
+std::string getAnyTargetForTesting() {
+ for (const auto &Target : llvm::TargetRegistry::targets()) {
+ std::string Error;
+ StringRef TargetName(Target.getName());
+ if (TargetName == "x86-64")
+ TargetName = "x86_64";
+ if (llvm::TargetRegistry::lookupTarget(std::string(TargetName), Error) ==
+ &Target) {
+ return std::string(TargetName);
+ }
+ }
+ return "";
+}
+
} // end namespace clang
Index: clang/include/clang/Tooling/Tooling.h
===================================================================
--- clang/include/clang/Tooling/Tooling.h
+++ clang/include/clang/Tooling/Tooling.h
@@ -506,6 +506,12 @@
void addTargetAndModeForProgramName(std::vector<std::string> &CommandLine,
StringRef InvokedAs);
+/// Helper function that expands response files in command line.
+void addExpandedResponseFiles(std::vector<std::string> &CommandLine,
+ llvm::StringRef WorkingDir,
+ llvm::cl::TokenizerCallback Tokenizer,
+ llvm::vfs::FileSystem &FS);
+
/// Creates a \c CompilerInvocation.
CompilerInvocation *newInvocation(DiagnosticsEngine *Diagnostics,
ArrayRef<const char *> CC1Args,
Index: clang/include/clang/Testing/CommandLineArgs.h
===================================================================
--- clang/include/clang/Testing/CommandLineArgs.h
+++ clang/include/clang/Testing/CommandLineArgs.h
@@ -38,6 +38,11 @@
StringRef getFilenameForTesting(TestLanguage Lang);
+/// Find a target name such that looking for it in TargetRegistry by that name
+/// returns the same target. We expect that there is at least one target
+/// configured with this property.
+std::string getAnyTargetForTesting();
+
} // end namespace clang
#endif
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -11,6 +11,7 @@
#include "TestFS.h"
#include "support/Context.h"
+#include "clang/Testing/CommandLineArgs.h"
#include "clang/Tooling/ArgumentsAdjusters.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
@@ -20,6 +21,7 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Process.h"
+#include "llvm/Support/TargetSelect.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -41,15 +43,18 @@
// Make use of all features and assert the exact command we get out.
// Other tests just verify presence/absence of certain args.
TEST(CommandMangler, Everything) {
+ llvm::InitializeAllTargetInfos(); // As in ClangdMain
+ std::string Target = getAnyTargetForTesting();
auto Mangler = CommandMangler::forTests();
Mangler.ClangPath = testPath("fake/clang");
Mangler.ResourceDir = testPath("fake/resources");
Mangler.Sysroot = testPath("fake/sysroot");
tooling::CompileCommand Cmd;
- Cmd.CommandLine = {"clang++", "--", "foo.cc", "bar.cc"};
+ Cmd.CommandLine = {Target + "-clang++", "--", "foo.cc", "bar.cc"};
Mangler(Cmd, "foo.cc");
EXPECT_THAT(Cmd.CommandLine,
- ElementsAre(testPath("fake/clang++"),
+ ElementsAre(testPath("fake/" + Target + "-clang++"),
+ "--target=" + Target, "--driver-mode=g++",
"-resource-dir=" + testPath("fake/resources"),
"-isysroot", testPath("fake/sysroot"), "--",
"foo.cc"));
@@ -197,8 +202,26 @@
Mangler(Cmd, "foo.cc");
}
// Edits are applied in given order and before other mangling and they always
- // go before filename.
- EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "--hello", "--", "FOO.CC"));
+ // go before filename. `--driver-mode=g++` here is in lower case because
+ // options inserted by addTargetAndModeForProgramName are not editable,
+ // see discussion in https://reviews.llvm.org/D138546
+ EXPECT_THAT(Cmd.CommandLine,
+ ElementsAre(_, "--driver-mode=g++", "--hello", "--", "FOO.CC"));
+}
+
+TEST(CommandMangler, ExpandedResponseFiles) {
+ SmallString<1024> Path;
+ int FD;
+ ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("args", "", FD, Path));
+ llvm::raw_fd_ostream OutStream(FD, true);
+ OutStream << "-Wall";
+ OutStream.close();
+
+ auto Mangler = CommandMangler::forTests();
+ tooling::CompileCommand Cmd;
+ Cmd.CommandLine = {"clang", ("@" + Path).str(), "foo.cc"};
+ Mangler(Cmd, "foo.cc");
+ EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "-Wall", "--", "foo.cc"));
}
static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -157,6 +157,7 @@
clangLex
clangSema
clangSerialization
+ clangTesting
clangTooling
clangToolingCore
clangToolingInclusions
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -244,15 +244,7 @@
parseJSON(PathRef Path, llvm::StringRef Data, std::string &Error) {
if (auto CDB = tooling::JSONCompilationDatabase::loadFromBuffer(
Data, Error, tooling::JSONCommandLineSyntax::AutoDetect)) {
- // FS used for expanding response files.
- // FIXME: ExpandResponseFilesDatabase appears not to provide the usual
- // thread-safety guarantees, as the access to FS is not locked!
- // For now, use the real FS, which is known to be threadsafe (if we don't
- // use/change working directory, which ExpandResponseFilesDatabase doesn't).
- auto FS = llvm::vfs::getRealFileSystem();
- return tooling::inferTargetAndDriverMode(
- tooling::inferMissingCompileCommands(
- expandResponseFiles(std::move(CDB), std::move(FS))));
+ return tooling::inferMissingCompileCommands(std::move(CDB));
}
return nullptr;
}
Index: clang-tools-extra/clangd/CompileCommands.h
===================================================================
--- clang-tools-extra/clangd/CompileCommands.h
+++ clang-tools-extra/clangd/CompileCommands.h
@@ -12,6 +12,7 @@
#include "support/Threading.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/CommandLine.h"
#include <deque>
#include <optional>
#include <string>
@@ -50,10 +51,11 @@
llvm::StringRef TargetFile) const;
private:
- CommandMangler() = default;
+ CommandMangler();
Memoize<llvm::StringMap<std::string>> ResolvedDrivers;
Memoize<llvm::StringMap<std::string>> ResolvedDriversNoFollow;
+ llvm::cl::TokenizerCallback Tokenizer;
};
// Removes args from a command-line in a semantically-aware way.
Index: clang-tools-extra/clangd/CompileCommands.cpp
===================================================================
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -14,6 +14,7 @@
#include "clang/Driver/Options.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
@@ -27,6 +28,7 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
+#include "llvm/TargetParser/Host.h"
#include <iterator>
#include <optional>
#include <string>
@@ -185,6 +187,12 @@
} // namespace
+CommandMangler::CommandMangler() {
+ Tokenizer = llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows()
+ ? llvm::cl::TokenizeWindowsCommandLine
+ : llvm::cl::TokenizeGNUCommandLine;
+}
+
CommandMangler CommandMangler::detect() {
CommandMangler Result;
Result.ClangPath = detectClangPath();
@@ -201,9 +209,18 @@
trace::Span S("AdjustCompileFlags");
// Most of the modifications below assumes the Cmd starts with a driver name.
// We might consider injecting a generic driver name like "cc" or "c++", but
- // a Cmd missing the driver is probably rare enough in practice and errnous.
+ // a Cmd missing the driver is probably rare enough in practice and erroneous.
if (Cmd.empty())
return;
+
+ // FS used for expanding response files.
+ // FIXME: ExpandResponseFiles appears not to provide the usual
+ // thread-safety guarantees, as the access to FS is not locked!
+ // For now, use the real FS, which is known to be threadsafe (if we don't
+ // use/change working directory, which ExpandResponseFiles doesn't).
+ auto FS = llvm::vfs::getRealFileSystem();
+ tooling::addExpandedResponseFiles(Cmd, Command.Directory, Tokenizer, *FS);
+
auto &OptTable = clang::driver::getDriverOptTable();
// OriginalArgs needs to outlive ArgList.
llvm::SmallVector<const char *, 16> OriginalArgs;
@@ -212,7 +229,7 @@
OriginalArgs.push_back(S.c_str());
bool IsCLMode = driver::IsClangCL(driver::getDriverMode(
OriginalArgs[0], llvm::ArrayRef(OriginalArgs).slice(1)));
- // ParseArgs propagates missig arg/opt counts on error, but preserves
+ // ParseArgs propagates missing arg/opt counts on error, but preserves
// everything it could parse in ArgList. So we just ignore those counts.
unsigned IgnoredCount;
// Drop the executable name, as ParseArgs doesn't expect it. This means
@@ -307,12 +324,16 @@
// necessary for the system include extractor to identify the file type
// - AFTER applying CompileFlags.Edits, because the name of the compiler
// that needs to be invoked may come from the CompileFlags->Compiler key
+ // - BEFORE addTargetAndModeForProgramName(), because gcc doesn't support
+ // the target flag that might be added.
// - BEFORE resolveDriver() because that can mess up the driver path,
// e.g. changing gcc to /path/to/clang/bin/gcc
if (SystemIncludeExtractor) {
SystemIncludeExtractor(Command, File);
}
+ tooling::addTargetAndModeForProgramName(Cmd, Cmd.front());
+
// Check whether the flag exists, either as -flag or -flag=*
auto Has = [&](llvm::StringRef Flag) {
for (llvm::StringRef Arg : Cmd) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
