[cfe-commits] [PATCH] Make CompilationDatabase work with symlinks and relative paths

[Daniel Jasper]

================
Comment at: include/clang/Tooling/CompilationDatabase.h:208-209
@@ +207,4 @@
+/// like this:
+///   /<project-root/src/<path>/<somefile>.cc      (used as input for the tool)
+///   /<project-root/build/<symlink-to-src>/<path>/<somefile>.cc (stored in DB)
+///
----------------
Chandler Carruth wrote:
> <project-root> is missing the '>' above.
Added.

================
Comment at: lib/Tooling/JSONCompilationDatabase.cpp:289
@@ +288,3 @@
+      llvm::SmallString<8> DirectoryStorage;
+      llvm::SmallString<128> AbsolutePath(Directory->getValue(DirectoryStorage));
+      llvm::sys::path::append(AbsolutePath, FileName);
----------------
Chandler Carruth wrote:
> is this line over 80 columns, or is phab just being confusing?
No, it was 81 columns.

================
Comment at: include/clang/Tooling/CompilationDatabase.h:193-198
@@ -188,1 +192,8 @@
 
+struct PathComparator {
+  virtual bool equivalent(const Twine &FileA, const Twine &FileB) const {
+    return FileA.str() == FileB.str() ||
+        llvm::sys::fs::equivalent(FileA, FileB);
+  }
+};
+
----------------
Chandler Carruth wrote:
> Why expose this and require it to be passed into findEquivalent? It would simplify things to sink this into the implementation details of findEquivalent. and hide its existence from the users.
Injecting this is important for testing and I did not want to store anything in the Trie nodes. I have now split findEquivalent into two methods, the public one requiring fewer arguments. I'll put more effort into hiding PathComparator once you okayed the design in general.

================
Comment at: include/clang/Tooling/CompilationDatabase.h:200-201
@@ +199,4 @@
+
+/// \brief A trie to efficiently match against the entries of the compilation
+/// database in order of matching suffix length.
+///
----------------
Chandler Carruth wrote:
> You explain in nice detail why this exists and its purpose, but no where do you really explain the datastructure technique you are using to build the trie efficiently. What are the expected storage requirements? what's the strategy used? I feel left a bit in the dark regarding the actual algorithms.
How about this?

================
Comment at: include/clang/Tooling/CompilationDatabase.h:236-239
@@ +235,6 @@
+  /// to 'Error'.
+  StringRef findEquivalent(const PathComparator &Comparator,
+                           StringRef FileName,
+                           std::string *Error,
+                           unsigned ConsumedLength = 0) const;
+
----------------
Chandler Carruth wrote:
> Rather than a "std::string *" output parameter, I would suggest passing in a reference to a raw_ostream. You can then build a raw_ostream around a std::string or whatever other string object you like when calling this, and the function can avail itself of all the streamed output conveniences.
Done.


http://llvm-reviews.chandlerc.com/D30