[cfe-commits] r154929 - in /cfe/trunk: include/clang/Tooling/CompilationDatabase.h lib/Tooling/CompilationDatabase.cpp
David Blaikie
dblaikie at gmail.com
Mon May 14 14:43:16 PDT 2012
Excuse my slightly delayed feedback,
On Tue, Apr 17, 2012 at 9:54 AM, Manuel Klimek <klimek at google.com> wrote:
> Author: klimek
> Date: Tue Apr 17 11:54:26 2012
> New Revision: 154929
>
> URL: http://llvm.org/viewvc/llvm-project?rev=154929&view=rev
> Log:
> Switches the JSONCompilationDatabase to use the YAML parser.
> This will allow us to delete the JSON parser from llvm.
>
> The biggest change is a general change of strategy - instead
> of storing StringRef's to the values for the command line and
> directory in the input buffer, we store ScalarNode*'s. The
> reason is that the YAML parser's getRawValue on ScalarNodes
> returns a string that includes the quotes in case of double
> quoted strings.
>
> For the same reason we're removing the JSON parsing part of
> the command line parsing - this means an extra copy for a
> command line when it is requested (and only when it is requested).
>
>
> Modified:
> cfe/trunk/include/clang/Tooling/CompilationDatabase.h
> cfe/trunk/lib/Tooling/CompilationDatabase.cpp
>
> Modified: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CompilationDatabase.h?rev=154929&r1=154928&r2=154929&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Tooling/CompilationDatabase.h (original)
> +++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h Tue Apr 17 11:54:26 2012
> @@ -34,13 +34,11 @@
> #include "llvm/ADT/StringMap.h"
> #include "llvm/ADT/StringRef.h"
> #include "llvm/Support/MemoryBuffer.h"
> +#include "llvm/Support/SourceMgr.h"
> +#include "llvm/Support/YAMLParser.h"
> #include <string>
> #include <vector>
>
> -namespace llvm {
> -class MemoryBuffer;
> -} // end namespace llvm
> -
> namespace clang {
> namespace tooling {
>
> @@ -139,7 +137,7 @@
> private:
> /// \brief Constructs a JSON compilation database on a memory buffer.
> JSONCompilationDatabase(llvm::MemoryBuffer *Database)
> - : Database(Database) {}
> + : Database(Database), YAMLStream(Database->getBuffer(), SM) {}
>
> /// \brief Parses the database file and creates the index.
> ///
> @@ -147,14 +145,17 @@
> /// failed.
> bool parse(std::string &ErrorMessage);
>
> - // Tuple (directory, commandline) where 'commandline' is a JSON escaped bash
> - // escaped command line.
> - typedef std::pair<StringRef, StringRef> CompileCommandRef;
> + // Tuple (directory, commandline) where 'commandline' pointing to the
> + // corresponding nodes in the YAML stream.
> + typedef std::pair<llvm::yaml::ScalarNode*,
> + llvm::yaml::ScalarNode*> CompileCommandRef;
>
> // Maps file paths to the compile command lines for that file.
> llvm::StringMap< std::vector<CompileCommandRef> > IndexByFile;
>
> llvm::OwningPtr<llvm::MemoryBuffer> Database;
> + llvm::SourceMgr SM;
> + llvm::yaml::Stream YAMLStream;
> };
>
> } // end namespace tooling
>
> Modified: cfe/trunk/lib/Tooling/CompilationDatabase.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CompilationDatabase.cpp?rev=154929&r1=154928&r2=154929&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Tooling/CompilationDatabase.cpp (original)
> +++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp Tue Apr 17 11:54:26 2012
> @@ -13,7 +13,7 @@
>
> #include "clang/Tooling/CompilationDatabase.h"
> #include "llvm/ADT/SmallString.h"
> -#include "llvm/Support/JSONParser.h"
> +#include "llvm/Support/YAMLParser.h"
> #include "llvm/Support/Path.h"
> #include "llvm/Support/system_error.h"
>
> @@ -22,10 +22,10 @@
>
> namespace {
>
> -/// \brief A parser for JSON escaped strings of command line arguments.
> +/// \brief A parser for escaped strings of command line arguments.
> ///
> /// Assumes \-escaping for quoted arguments (see the documentation of
> -/// unescapeJSONCommandLine(...)).
> +/// unescapeCommandLine(...)).
> class CommandLineArgumentParser {
> public:
> CommandLineArgumentParser(StringRef CommandLine)
> @@ -90,9 +90,6 @@
>
> bool next() {
> ++Position;
> - if (Position == Input.end()) return false;
> - // Remove the JSON escaping first. This is done unconditionally.
> - if (*Position == '\\') ++Position;
> return Position != Input.end();
> }
>
> @@ -101,9 +98,9 @@
> std::vector<std::string> CommandLine;
> };
>
> -std::vector<std::string> unescapeJSONCommandLine(
> - StringRef JSONEscapedCommandLine) {
> - CommandLineArgumentParser parser(JSONEscapedCommandLine);
> +std::vector<std::string> unescapeCommandLine(
> + StringRef EscapedCommandLine) {
> + CommandLineArgumentParser parser(EscapedCommandLine);
> return parser.parse();
> }
>
> @@ -162,65 +159,77 @@
> const std::vector<CompileCommandRef> &CommandsRef = CommandsRefI->getValue();
> std::vector<CompileCommand> Commands;
> for (int I = 0, E = CommandsRef.size(); I != E; ++I) {
> + llvm::SmallString<8> DirectoryStorage;
> + llvm::SmallString<1024> CommandStorage;
> Commands.push_back(CompileCommand(
> // FIXME: Escape correctly:
> - CommandsRef[I].first,
> - unescapeJSONCommandLine(CommandsRef[I].second)));
> + CommandsRef[I].first->getValue(DirectoryStorage),
> + unescapeCommandLine(CommandsRef[I].second->getValue(CommandStorage))));
> }
> return Commands;
> }
>
> bool JSONCompilationDatabase::parse(std::string &ErrorMessage) {
> - llvm::SourceMgr SM;
> - llvm::JSONParser Parser(Database->getBuffer(), &SM);
> - llvm::JSONValue *Root = Parser.parseRoot();
> + llvm::yaml::document_iterator I = YAMLStream.begin();
> + if (I == YAMLStream.end()) {
> + ErrorMessage = "Error while parsing YAML.";
> + return false;
> + }
> + llvm::yaml::Node *Root = I->getRoot();
> if (Root == NULL) {
> - ErrorMessage = "Error while parsing JSON.";
> + ErrorMessage = "Error while parsing YAML.";
> return false;
> }
> - llvm::JSONArray *Array = dyn_cast<llvm::JSONArray>(Root);
> + llvm::yaml::SequenceNode *Array =
> + llvm::dyn_cast<llvm::yaml::SequenceNode>(Root);
> if (Array == NULL) {
> ErrorMessage = "Expected array.";
> return false;
> }
> - for (llvm::JSONArray::const_iterator AI = Array->begin(), AE = Array->end();
> + for (llvm::yaml::SequenceNode::iterator AI = Array->begin(),
> + AE = Array->end();
> AI != AE; ++AI) {
> - const llvm::JSONObject *Object = dyn_cast<llvm::JSONObject>(*AI);
> + llvm::yaml::MappingNode *Object =
> + llvm::dyn_cast<llvm::yaml::MappingNode>(&*AI);
> if (Object == NULL) {
> ErrorMessage = "Expected object.";
> return false;
> }
> - StringRef EntryDirectory;
> - StringRef EntryFile;
> - StringRef EntryCommand;
> - for (llvm::JSONObject::const_iterator KVI = Object->begin(),
> - KVE = Object->end();
> + llvm::yaml::ScalarNode *Directory;
> + llvm::yaml::ScalarNode *Command;
> + llvm::SmallString<8> FileStorage;
> + llvm::StringRef File;
> + for (llvm::yaml::MappingNode::iterator KVI = Object->begin(),
> + KVE = Object->end();
> KVI != KVE; ++KVI) {
> - const llvm::JSONValue *Value = (*KVI)->Value;
> + llvm::yaml::Node *Value = (*KVI).getValue();
> if (Value == NULL) {
> ErrorMessage = "Expected value.";
> return false;
> }
> - const llvm::JSONString *ValueString =
> - dyn_cast<llvm::JSONString>(Value);
> + llvm::yaml::ScalarNode *ValueString =
> + llvm::dyn_cast<llvm::yaml::ScalarNode>(Value);
Should this be a (non-dyn) llvm::cast? Or should it be null checked
before it's dereferenced 5 lines down?
> if (ValueString == NULL) {
> ErrorMessage = "Expected string as value.";
> return false;
> }
> - if ((*KVI)->Key->getRawText() == "directory") {
> - EntryDirectory = ValueString->getRawText();
> - } else if ((*KVI)->Key->getRawText() == "file") {
> - EntryFile = ValueString->getRawText();
> - } else if ((*KVI)->Key->getRawText() == "command") {
> - EntryCommand = ValueString->getRawText();
> + llvm::yaml::ScalarNode *KeyString =
> + llvm::dyn_cast<llvm::yaml::ScalarNode>((*KVI).getKey());
> + llvm::SmallString<8> KeyStorage;
> + if (KeyString->getValue(KeyStorage) == "directory") {
> + Directory = ValueString;
> + } else if (KeyString->getValue(KeyStorage) == "command") {
> + Command = ValueString;
> + } else if (KeyString->getValue(KeyStorage) == "file") {
> + File = ValueString->getValue(FileStorage);
> } else {
> - ErrorMessage = (Twine("Unknown key: \"") +
> - (*KVI)->Key->getRawText() + "\"").str();
> + ErrorMessage = ("Unknown key: \"" +
> + KeyString->getRawValue() + "\"").str();
> return false;
> }
> }
> - IndexByFile[EntryFile].push_back(
> - CompileCommandRef(EntryDirectory, EntryCommand));
> + IndexByFile[File].push_back(
> + CompileCommandRef(Directory, Command));
Are 'Directory' and 'Command' guaranteed to have been initialized by
the above loop? (perhaps the file has been verified to contain a
'directory' and 'command' key by some previous code? I haven't looked
closely)
GCC's maybe-uninitialized mentioned this so I just thought I'd have a
look around at it.
- David
> }
> return true;
> }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list