Extend CompilationDatabase by a field for the output filename

[joerg]

Summary

In bigger projects like an Operating System, the same source code is often compiled in slightly different ways. This could be the difference between PIC and non-PIC code for static vs dynamic libraries, it could also be the difference between size optimised versions of tools for ramdisk images. At the moment, the compilation database has no way to distinguish such cases. As first step, add a field in the JSON format for it and process it accordingly.

[klimek]

lib/Tooling/JSONCompilationDatabase.cpp

266 ↗

(On Diff #79320)

Optional: I'd probably let the nodeToCommandLine handle the null value and make this code more straight forward?

[joerg]

lib/Tooling/JSONCompilationDatabase.cpp

266 ↗

(On Diff #79320)

I couldn't find a way to create a synthetic node without changing the YAML API.

[klimek]

lib/Tooling/JSONCompilationDatabase.cpp

266 ↗

(On Diff #79320)

I'm probably missing something - why would we need a synthetic node? Can't we just put nullptr into the vector?

[joerg]

lib/Tooling/JSONCompilationDatabase.cpp

266 ↗

(On Diff #79320)

That's what I am doing and why this line checks output :)

[klimek]

lib/Tooling/JSONCompilationDatabase.cpp

266 ↗

(On Diff #79320)

Ok, let's ask differently: why is it a problem if we put a nullptr into the array?

[joerg]

lib/Tooling/JSONCompilationDatabase.cpp

266 ↗

(On Diff #79320)

I think it just adds unnecessary complexity. An empty file name is not a valid output, so "" vs Optional has the same result. I'd have prefered to keep this complexity inside the JSON parser, but that would have meant creating a synthetic node with value "" and there is no API for that at the moment.

[joerg]

Can I interprete that as LGTM otherwise?

[klimek]

lib/Tooling/JSONCompilationDatabase.cpp

266 ↗

(On Diff #79320)

Don't we thus want to actually be able to err out on a higher level when getting an empty output dir, but be OK with an unspecified (-> nullptr) one?

[joerg]

It's not the directory, but the output file. That's optional since it is a new addition and I don't want to invalidate all existing JSON databases.

[klimek]

In D27138#607786, @joerg wrote:

It's not the directory, but the output file. That's optional since it is a new addition and I don't want to invalidate all existing JSON databases.

It seems like we're talking past each other. I'm not suggesting to invalidate all existing JSON databases. I'm suggesting:

1. if it doesn't exist, have nullptr in the struct
2. if it exists and is empty (""), have the empty string in the struct (and potentially give the user an error)
3. if it exists and is non-empty, all is good

[joerg]

Which struct are we talking about, CompileCommandRef or CompileCommand? It is a pointer in the former and a plain StringRef in the latter. I don't think making it a pointer in both is an advantage, i.e. distinguishing empty input from missing field is not valuable in my opinion.

[klimek]

In D27138#607859, @joerg wrote:

Which struct are we talking about, CompileCommandRef or CompileCommand? It is a pointer in the former and a plain StringRef in the latter. I don't think making it a pointer in both is an advantage, i.e. distinguishing empty input from missing field is not valuable in my opinion.

Ok, you convinced me. LG then.

cfe/trunk/docs/JSONCompilationDatabase.rst

 -  **command:** The compile command executed. After JSON unescaping,
    this must be a valid command to rerun the exact compilation step for
    the translation unit in the environment the build system uses.
    Parameters use shell quoting and shell escaping of quotes, with '``"``'
    and '``\``' being the only special characters. Shell expansion is not
    supported.
 -  **arguments:** The compile command executed as list of strings.
    Either **arguments** or **command** is required.
+-  **output:** The name of the output created by this compilation step.
+   This field is optional. It can be used to distinguish different processing
+   modes of the same input file.
 
 Build System Integration
 ========================
 
 The convention is to name the file compile\_commands.json and put it at
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.

cfe/trunk/include/clang/Tooling/CompilationDatabase.h

 
 namespace clang {
 namespace tooling {
 
 /// \brief Specifies the working directory and command of a compilation.
 struct CompileCommand {
   CompileCommand() {}
   CompileCommand(Twine Directory, Twine Filename,
-                 std::vector<std::string> CommandLine)
+                 std::vector<std::string> CommandLine, Twine Output)
       : Directory(Directory.str()),
         Filename(Filename.str()),
-        CommandLine(std::move(CommandLine)) {}
+        CommandLine(std::move(CommandLine)),
+        Output(Output.str()){}
 
   /// \brief The working directory the command was executed from.
   std::string Directory;
 
   /// The source file associated with the command.
   std::string Filename;
 
   /// \brief The command line that was executed.
   std::vector<std::string> CommandLine;
 
+  /// The output file associated with the command.
+  std::string Output;
+
   /// \brief An optional mapping from each file's path to its content for all
   /// files needed for the compilation that are not available via the file
   /// system.
   ///
   /// Note that a tool implementation is required to fall back to the file
   /// system if a source file is not provided in the mapped sources, as
   /// compilation databases will usually not provide all files in mapped sources
   /// for performance reasons.

cfe/trunk/include/clang/Tooling/JSONCompilationDatabase.h

JSONCompilationDatabase(std::unique_ptr<llvm::MemoryBuffer> Database,
         YAMLStream(this->Database->getBuffer(), SM) {}
 
   /// \brief Parses the database file and creates the index.
   ///
   /// Returns whether parsing succeeded. Sets ErrorMessage if parsing
   /// failed.
   bool parse(std::string &ErrorMessage);
 
-  // Tuple (directory, filename, commandline) where 'commandline' points to the
-  // corresponding scalar nodes in the YAML stream.
+  // Tuple (directory, filename, commandline, output) where 'commandline'
+  // points to the corresponding scalar nodes in the YAML stream.
   // If the command line contains a single argument, it is a shell-escaped
   // command line.
   // Otherwise, each entry in the command line vector is a literal
   // argument to the compiler.
+  // The output field may be a nullptr.
   typedef std::tuple<llvm::yaml::ScalarNode *,
                      llvm::yaml::ScalarNode *,
-                    std::vector<llvm::yaml::ScalarNode *>> CompileCommandRef;
+                     std::vector<llvm::yaml::ScalarNode *>,
+                     llvm::yaml::ScalarNode *> CompileCommandRef;
 
   /// \brief Converts the given array of CompileCommandRefs to CompileCommands.
   void getCommands(ArrayRef<CompileCommandRef> CommandsRef,
                    std::vector<CompileCommand> &Commands) const;
 
   // Maps file paths to the compile command lines for that file.
   llvm::StringMap<std::vector<CompileCommandRef>> IndexByFile;
 

cfe/trunk/lib/Tooling/CompilationDatabase.cpp

 }
 
 FixedCompilationDatabase::
 FixedCompilationDatabase(Twine Directory, ArrayRef<std::string> CommandLine) {
   std::vector<std::string> ToolCommandLine(1, "clang-tool");
   ToolCommandLine.insert(ToolCommandLine.end(),
                          CommandLine.begin(), CommandLine.end());
   CompileCommands.emplace_back(Directory, StringRef(),
-                               std::move(ToolCommandLine));
+                               std::move(ToolCommandLine),
+                               StringRef());
 }
 
 std::vector<CompileCommand>
 FixedCompilationDatabase::getCompileCommands(StringRef FilePath) const {
   std::vector<CompileCommand> Result(CompileCommands);
   Result[0].CommandLine.push_back(FilePath);
   Result[0].Filename = FilePath;
   return Result;

cfe/trunk/lib/Tooling/JSONCompilationDatabase.cpp

 }
 
 void JSONCompilationDatabase::getCommands(
     ArrayRef<CompileCommandRef> CommandsRef,
     std::vector<CompileCommand> &Commands) const {
   for (int I = 0, E = CommandsRef.size(); I != E; ++I) {
     SmallString<8> DirectoryStorage;
     SmallString<32> FilenameStorage;
+    SmallString<32> OutputStorage;
+    auto Output = std::get<3>(CommandsRef[I]);
     Commands.emplace_back(
         std::get<0>(CommandsRef[I])->getValue(DirectoryStorage),
         std::get<1>(CommandsRef[I])->getValue(FilenameStorage),
-        nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])));
+        nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])),
+        Output ? Output->getValue(OutputStorage) : "");
   }
 }
 
 bool JSONCompilationDatabase::parse(std::string &ErrorMessage) {
   llvm::yaml::document_iterator I = YAMLStream.begin();
   if (I == YAMLStream.end()) {
     ErrorMessage = "Error while parsing YAML.";
     return false;
for (auto& NextObject : *Array) {
     llvm::yaml::MappingNode *Object = dyn_cast<llvm::yaml::MappingNode>(&NextObject);
     if (!Object) {
       ErrorMessage = "Expected object.";
       return false;
     }
     llvm::yaml::ScalarNode *Directory = nullptr;
     llvm::Optional<std::vector<llvm::yaml::ScalarNode *>> Command;
     llvm::yaml::ScalarNode *File = nullptr;
+    llvm::yaml::ScalarNode *Output = nullptr;
     for (auto& NextKeyValue : *Object) {
       llvm::yaml::ScalarNode *KeyString =
           dyn_cast<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
       if (!KeyString) {
         ErrorMessage = "Expected strings as key.";
         return false;
       }
       SmallString<10> KeyStorage;
for (auto& NextKeyValue : *Object) {
           }
           Command->push_back(Scalar);
         }
       } else if (KeyValue == "command") {
         if (!Command)
           Command = std::vector<llvm::yaml::ScalarNode *>(1, ValueString);
       } else if (KeyValue == "file") {
         File = ValueString;
+      } else if (KeyValue == "output") {
+        Output = ValueString;
       } else {
         ErrorMessage = ("Unknown key: \"" +
                         KeyString->getRawValue() + "\"").str();
         return false;
       }
     }
     if (!File) {
       ErrorMessage = "Missing key: \"file\".";
if (llvm::sys::path::is_relative(FileName)) {
       SmallString<8> DirectoryStorage;
       SmallString<128> AbsolutePath(
           Directory->getValue(DirectoryStorage));
       llvm::sys::path::append(AbsolutePath, FileName);
       llvm::sys::path::native(AbsolutePath, NativeFilePath);
     } else {
       llvm::sys::path::native(FileName, NativeFilePath);
     }
-    auto Cmd = CompileCommandRef(Directory, File, *Command);
+    auto Cmd = CompileCommandRef(Directory, File, *Command, Output);
     IndexByFile[NativeFilePath].push_back(Cmd);
     AllCommands.push_back(Cmd);
     MatchTrie.insert(NativeFilePath);
   }
   return true;
 }
 
 } // end namespace tooling
 } // end namespace clang

cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp

TEST(JSONCompilationDatabase, ErrsOnInvalidFormat) {
   expectFailure("[{\"directory\":\"\",\"file\":\"\"}]", "Missing command or arguments");
   expectFailure("[{\"command\":\"\",\"file\":\"\"}]", "Missing directory");
   expectFailure("[{\"directory\":\"\",\"arguments\":[]}]", "Missing file");
   expectFailure("[{\"arguments\":\"\",\"file\":\"\"}]", "Missing directory");
   expectFailure("[{\"directory\":\"\",\"arguments\":\"\",\"file\":\"\"}]", "Arguments not array");
   expectFailure("[{\"directory\":\"\",\"command\":[],\"file\":\"\"}]", "Command not string");
   expectFailure("[{\"directory\":\"\",\"arguments\":[[]],\"file\":\"\"}]",
                 "Arguments contain non-string");
+  expectFailure("[{\"output\":[]}]", "Expected strings as value.");
 }
 
 static std::vector<std::string> getAllFiles(StringRef JSONDatabase,
                                             std::string &ErrorMessage,
                                             JSONCommandLineSyntax Syntax) {
   std::unique_ptr<CompilationDatabase> Database(
       JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage,
                                               Syntax));
TEST(JSONCompilationDatabase, GetAllCompileCommands) {
   EXPECT_EQ(
       0u, getAllCompileCommands(JSONCommandLineSyntax::Gnu, "[]", ErrorMessage)
               .size())
       << ErrorMessage;
 
   StringRef Directory1("//net/dir1");
   StringRef FileName1("file1");
   StringRef Command1("command1");
+  StringRef Output1("file1.o");
   StringRef Directory2("//net/dir2");
   StringRef FileName2("file2");
   StringRef Command2("command2");
+  StringRef Output2("");
 
   std::vector<CompileCommand> Commands = getAllCompileCommands(
       JSONCommandLineSyntax::Gnu,
       ("[{\"directory\":\"" + Directory1 + "\"," + "\"command\":\"" + Command1 +
        "\","
        "\"file\":\"" +
-       FileName1 + "\"},"
+       FileName1 + "\", \"output\":\"" +
+       Output1 + "\"},"
                    " {\"directory\":\"" +
        Directory2 + "\"," + "\"command\":\"" + Command2 + "\","
                                                           "\"file\":\"" +
        FileName2 + "\"}]")
           .str(),
       ErrorMessage);
   EXPECT_EQ(2U, Commands.size()) << ErrorMessage;
   EXPECT_EQ(Directory1, Commands[0].Directory) << ErrorMessage;
   EXPECT_EQ(FileName1, Commands[0].Filename) << ErrorMessage;
+  EXPECT_EQ(Output1, Commands[0].Output) << ErrorMessage;
   ASSERT_EQ(1u, Commands[0].CommandLine.size());
   EXPECT_EQ(Command1, Commands[0].CommandLine[0]) << ErrorMessage;
   EXPECT_EQ(Directory2, Commands[1].Directory) << ErrorMessage;
   EXPECT_EQ(FileName2, Commands[1].Filename) << ErrorMessage;
+  EXPECT_EQ(Output2, Commands[1].Output) << ErrorMessage;
   ASSERT_EQ(1u, Commands[1].CommandLine.size());
   EXPECT_EQ(Command2, Commands[1].CommandLine[0]) << ErrorMessage;
 
   // Check that order is preserved.
   Commands = getAllCompileCommands(
       JSONCommandLineSyntax::Gnu,
       ("[{\"directory\":\"" + Directory2 + "\"," + "\"command\":\"" + Command2 +
        "\","