================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
