zturner created this revision. zturner added a reviewer: alexfh. zturner added a subscriber: cfe-commits. Herald added a subscriber: klimek.
After submitting the windows command line parsing patch, it broke all the command line parsing unittests, because now they were trying parse gnu command lines using the windows command line parser. The patch is reverted for now until we get these unit tests fixed. I'm not sure what the best solution is. One option is to disable those tests on Windows. But that seems undesirable because it was only some of the tests, and others worked fine. Not all the tests even depended on a command line in the first place. So I decided to expose the command syntax through the API. You can either force it to Gnu, force it to Windows, and let it auto-detect. The unit tests all force it to Gnu for the sake of simplicity, since the Windows command line tokenizer is already rigorously tested in LLVM. This also kind of lines up with the idea I had in the previous code review for exposing a command line option in `CommonOptions.cpp` to override the command syntax so we don't have to guess. So if someone decides to do that in the future that should be easier now. https://reviews.llvm.org/D23628 Files: include/clang/Tooling/JSONCompilationDatabase.h lib/Tooling/JSONCompilationDatabase.cpp unittests/Tooling/CompilationDatabaseTest.cpp
Index: unittests/Tooling/CompilationDatabaseTest.cpp =================================================================== --- unittests/Tooling/CompilationDatabaseTest.cpp +++ unittests/Tooling/CompilationDatabaseTest.cpp @@ -21,9 +21,10 @@ static void expectFailure(StringRef JSONDatabase, StringRef Explanation) { std::string ErrorMessage; - EXPECT_EQ(nullptr, JSONCompilationDatabase::loadFromBuffer(JSONDatabase, - ErrorMessage)) - << "Expected an error because of: " << Explanation.str(); + EXPECT_EQ(nullptr, + JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage, + JSONCommandLineSyntax::Gnu)) + << "Expected an error because of: " << Explanation.str(); } TEST(JSONCompilationDatabase, ErrsOnInvalidFormat) { @@ -46,20 +47,24 @@ } static std::vector<std::string> getAllFiles(StringRef JSONDatabase, - std::string &ErrorMessage) { + std::string &ErrorMessage, + JSONCommandLineSyntax Syntax) { std::unique_ptr<CompilationDatabase> Database( - JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage)); + JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage, + Syntax)); if (!Database) { ADD_FAILURE() << ErrorMessage; return std::vector<std::string>(); } return Database->getAllFiles(); } -static std::vector<CompileCommand> getAllCompileCommands(StringRef JSONDatabase, - std::string &ErrorMessage) { +static std::vector<CompileCommand> +getAllCompileCommands(JSONCommandLineSyntax Syntax, StringRef JSONDatabase, + std::string &ErrorMessage) { std::unique_ptr<CompilationDatabase> Database( - JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage)); + JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage, + Syntax)); if (!Database) { ADD_FAILURE() << ErrorMessage; return std::vector<CompileCommand>(); @@ -70,28 +75,32 @@ TEST(JSONCompilationDatabase, GetAllFiles) { std::string ErrorMessage; EXPECT_EQ(std::vector<std::string>(), - getAllFiles("[]", ErrorMessage)) << ErrorMessage; + getAllFiles("[]", ErrorMessage, JSONCommandLineSyntax::Gnu)) + << ErrorMessage; std::vector<std::string> expected_files; SmallString<16> PathStorage; llvm::sys::path::native("//net/dir/file1", PathStorage); expected_files.push_back(PathStorage.str()); llvm::sys::path::native("//net/dir/file2", PathStorage); expected_files.push_back(PathStorage.str()); - EXPECT_EQ(expected_files, getAllFiles( - "[{\"directory\":\"//net/dir\"," - "\"command\":\"command\"," - "\"file\":\"file1\"}," - " {\"directory\":\"//net/dir\"," - "\"command\":\"command\"," - "\"file\":\"file2\"}]", - ErrorMessage)) << ErrorMessage; + EXPECT_EQ(expected_files, + getAllFiles("[{\"directory\":\"//net/dir\"," + "\"command\":\"command\"," + "\"file\":\"file1\"}," + " {\"directory\":\"//net/dir\"," + "\"command\":\"command\"," + "\"file\":\"file2\"}]", + ErrorMessage, JSONCommandLineSyntax::Gnu)) + << ErrorMessage; } TEST(JSONCompilationDatabase, GetAllCompileCommands) { std::string ErrorMessage; - EXPECT_EQ(0u, - getAllCompileCommands("[]", ErrorMessage).size()) << ErrorMessage; + EXPECT_EQ( + 0u, getAllCompileCommands(JSONCommandLineSyntax::Gnu, "[]", ErrorMessage) + .size()) + << ErrorMessage; StringRef Directory1("//net/dir1"); StringRef FileName1("file1"); @@ -101,12 +110,16 @@ StringRef Command2("command2"); std::vector<CompileCommand> Commands = getAllCompileCommands( - ("[{\"directory\":\"" + Directory1 + "\"," + - "\"command\":\"" + Command1 + "\"," - "\"file\":\"" + FileName1 + "\"}," - " {\"directory\":\"" + Directory2 + "\"," + - "\"command\":\"" + Command2 + "\"," - "\"file\":\"" + FileName2 + "\"}]").str(), + JSONCommandLineSyntax::Gnu, + ("[{\"directory\":\"" + Directory1 + "\"," + "\"command\":\"" + Command1 + + "\"," + "\"file\":\"" + + FileName1 + "\"}," + " {\"directory\":\"" + + Directory2 + "\"," + "\"command\":\"" + Command2 + "\"," + "\"file\":\"" + + FileName2 + "\"}]") + .str(), ErrorMessage); EXPECT_EQ(2U, Commands.size()) << ErrorMessage; EXPECT_EQ(Directory1, Commands[0].Directory) << ErrorMessage; @@ -120,12 +133,16 @@ // Check that order is preserved. Commands = getAllCompileCommands( - ("[{\"directory\":\"" + Directory2 + "\"," + - "\"command\":\"" + Command2 + "\"," - "\"file\":\"" + FileName2 + "\"}," - " {\"directory\":\"" + Directory1 + "\"," + - "\"command\":\"" + Command1 + "\"," - "\"file\":\"" + FileName1 + "\"}]").str(), + JSONCommandLineSyntax::Gnu, + ("[{\"directory\":\"" + Directory2 + "\"," + "\"command\":\"" + Command2 + + "\"," + "\"file\":\"" + + FileName2 + "\"}," + " {\"directory\":\"" + + Directory1 + "\"," + "\"command\":\"" + Command1 + "\"," + "\"file\":\"" + + FileName1 + "\"}]") + .str(), ErrorMessage); EXPECT_EQ(2U, Commands.size()) << ErrorMessage; EXPECT_EQ(Directory2, Commands[0].Directory) << ErrorMessage; @@ -142,7 +159,8 @@ StringRef JSONDatabase, std::string &ErrorMessage) { std::unique_ptr<CompilationDatabase> Database( - JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage)); + JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage, + JSONCommandLineSyntax::Gnu)); if (!Database) return CompileCommand(); std::vector<CompileCommand> Commands = Database->getCompileCommands(FileName); Index: lib/Tooling/JSONCompilationDatabase.cpp =================================================================== --- lib/Tooling/JSONCompilationDatabase.cpp +++ lib/Tooling/JSONCompilationDatabase.cpp @@ -16,7 +16,10 @@ #include "clang/Tooling/CompilationDatabasePluginRegistry.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/SmallString.h" +#include "llvm/Support/Allocator.h" +#include "llvm/Support/CommandLine.h" #include "llvm/Support/Path.h" +#include "llvm/Support/StringSaver.h" #include <system_error> namespace clang { @@ -111,8 +114,31 @@ std::vector<std::string> CommandLine; }; -std::vector<std::string> unescapeCommandLine( - StringRef EscapedCommandLine) { +std::vector<std::string> unescapeCommandLine(JSONCommandLineSyntax Syntax, + StringRef EscapedCommandLine) { + if (Syntax == JSONCommandLineSyntax::AutoDetect) { + llvm::Triple Triple(llvm::sys::getProcessTriple()); + if (Triple.getOS() == llvm::Triple::OSType::Win32) { + // Assume Windows command line parsing on Win32 unless the triple + // explicitly + // tells us otherwise. + if (!Triple.hasEnvironment() || + Triple.getEnvironment() == llvm::Triple::EnvironmentType::MSVC) + Syntax = JSONCommandLineSyntax::Windows; + else + Syntax = JSONCommandLineSyntax::Gnu; + } + } + + if (Syntax == JSONCommandLineSyntax::Windows) { + llvm::BumpPtrAllocator Alloc; + llvm::StringSaver Saver(Alloc); + llvm::SmallVector<const char *, 64> T; + llvm::cl::TokenizeWindowsCommandLine(EscapedCommandLine, Saver, T); + std::vector<std::string> Result(T.begin(), T.end()); + return Result; + } + assert(Syntax == JSONCommandLineSyntax::Gnu); CommandLineArgumentParser parser(EscapedCommandLine); return parser.parse(); } @@ -123,7 +149,8 @@ SmallString<1024> JSONDatabasePath(Directory); llvm::sys::path::append(JSONDatabasePath, "compile_commands.json"); std::unique_ptr<CompilationDatabase> Database( - JSONCompilationDatabase::loadFromFile(JSONDatabasePath, ErrorMessage)); + JSONCompilationDatabase::loadFromFile( + JSONDatabasePath, ErrorMessage, JSONCommandLineSyntax::AutoDetect)); if (!Database) return nullptr; return Database; @@ -143,27 +170,29 @@ std::unique_ptr<JSONCompilationDatabase> JSONCompilationDatabase::loadFromFile(StringRef FilePath, - std::string &ErrorMessage) { + std::string &ErrorMessage, + JSONCommandLineSyntax Syntax) { llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> DatabaseBuffer = llvm::MemoryBuffer::getFile(FilePath); if (std::error_code Result = DatabaseBuffer.getError()) { ErrorMessage = "Error while opening JSON database: " + Result.message(); return nullptr; } std::unique_ptr<JSONCompilationDatabase> Database( - new JSONCompilationDatabase(std::move(*DatabaseBuffer))); + new JSONCompilationDatabase(std::move(*DatabaseBuffer), Syntax)); if (!Database->parse(ErrorMessage)) return nullptr; return Database; } std::unique_ptr<JSONCompilationDatabase> JSONCompilationDatabase::loadFromBuffer(StringRef DatabaseString, - std::string &ErrorMessage) { + std::string &ErrorMessage, + JSONCommandLineSyntax Syntax) { std::unique_ptr<llvm::MemoryBuffer> DatabaseBuffer( llvm::MemoryBuffer::getMemBuffer(DatabaseString)); std::unique_ptr<JSONCompilationDatabase> Database( - new JSONCompilationDatabase(std::move(DatabaseBuffer))); + new JSONCompilationDatabase(std::move(DatabaseBuffer), Syntax)); if (!Database->parse(ErrorMessage)) return nullptr; return Database; @@ -211,10 +240,11 @@ } static std::vector<std::string> -nodeToCommandLine(const std::vector<llvm::yaml::ScalarNode *> &Nodes) { +nodeToCommandLine(JSONCommandLineSyntax Syntax, + const std::vector<llvm::yaml::ScalarNode *> &Nodes) { SmallString<1024> Storage; if (Nodes.size() == 1) { - return unescapeCommandLine(Nodes[0]->getValue(Storage)); + return unescapeCommandLine(Syntax, Nodes[0]->getValue(Storage)); } std::vector<std::string> Arguments; for (auto *Node : Nodes) { @@ -230,9 +260,9 @@ SmallString<8> DirectoryStorage; SmallString<32> FilenameStorage; Commands.emplace_back( - std::get<0>(CommandsRef[I])->getValue(DirectoryStorage), - std::get<1>(CommandsRef[I])->getValue(FilenameStorage), - nodeToCommandLine(std::get<2>(CommandsRef[I]))); + std::get<0>(CommandsRef[I])->getValue(DirectoryStorage), + std::get<1>(CommandsRef[I])->getValue(FilenameStorage), + nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I]))); } } Index: include/clang/Tooling/JSONCompilationDatabase.h =================================================================== --- include/clang/Tooling/JSONCompilationDatabase.h +++ include/clang/Tooling/JSONCompilationDatabase.h @@ -55,20 +55,23 @@ /// /// JSON compilation databases can for example be generated in CMake projects /// by setting the flag -DCMAKE_EXPORT_COMPILE_COMMANDS. +enum class JSONCommandLineSyntax { Windows, Gnu, AutoDetect }; class JSONCompilationDatabase : public CompilationDatabase { public: /// \brief Loads a JSON compilation database from the specified file. /// /// Returns NULL and sets ErrorMessage if the database could not be /// loaded from the given file. static std::unique_ptr<JSONCompilationDatabase> - loadFromFile(StringRef FilePath, std::string &ErrorMessage); + loadFromFile(StringRef FilePath, std::string &ErrorMessage, + JSONCommandLineSyntax Syntax); /// \brief Loads a JSON compilation database from a data buffer. /// /// Returns NULL and sets ErrorMessage if the database could not be loaded. static std::unique_ptr<JSONCompilationDatabase> - loadFromBuffer(StringRef DatabaseString, std::string &ErrorMessage); + loadFromBuffer(StringRef DatabaseString, std::string &ErrorMessage, + JSONCommandLineSyntax Syntax); /// \brief Returns all compile comamnds in which the specified file was /// compiled. @@ -89,8 +92,9 @@ private: /// \brief Constructs a JSON compilation database on a memory buffer. - JSONCompilationDatabase(std::unique_ptr<llvm::MemoryBuffer> Database) - : Database(std::move(Database)), + JSONCompilationDatabase(std::unique_ptr<llvm::MemoryBuffer> Database, + JSONCommandLineSyntax Syntax) + : Database(std::move(Database)), Syntax(Syntax), YAMLStream(this->Database->getBuffer(), SM) {} /// \brief Parses the database file and creates the index. @@ -123,6 +127,7 @@ FileMatchTrie MatchTrie; std::unique_ptr<llvm::MemoryBuffer> Database; + JSONCommandLineSyntax Syntax; llvm::SourceMgr SM; llvm::yaml::Stream YAMLStream; };
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits