ilya-biryukov added inline comments.
================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:26
+// their compile commands. If getAllFilenames() is empty, no inference occurs.
+//
+//===----------------------------------------------------------------------===//
----------------
Maybe add a comment describing the use-cases we had in mind while designing
these heuristics?
- .cpp and .h files usually have the same (or slightly modified) name,
usually the prefix match,
- LLVM (and other) codebases that put .h and .cpp files into different
directories,
- even random matches are better than arbitrary defaults,
- ...
================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:83
+template <bool Reverse> int prefixCompare(StringRef S, StringRef Prefix) {
+ if (S.size() >= Prefix.size())
+ return Reverse ? rMemCompare(S.end(), Prefix.end(), Prefix.size())
----------------
Summarizing the offline discussion, we could exclude suffix matches from the
initial version. This would make the code much simpler, and it seems most C++
projects we know of would actually work with prefix-only matches.
================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:115
+ for (auto F : getAllFiles())
+ Paths.emplace_back(Strings.save(F), 0);
+ finalizeIndex();
----------------
This class seems to do two somewhat orthogonal things:
- build and query the index structure for the paths,
- handle queries to inner CDB and patch the compile commands for other files
accordingly.
Maybe we could extract the code that handles the index into a separate class?
================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:141
+ void finalizeIndex() {
+ llvm::sort(Paths.begin(), Paths.end());
+ for (size_t I = 0; I < Paths.size(); ++I) {
----------------
Maybe store lower-cased paths in the index and compare case-insensitively when
querying?
Having slight case mismatches is not uncommon and case-sensitivity shouldn't
ever be the defining factor for this kind of heuristics.
================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:147
+ auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
+ // Index up to 4 path components.
+ for (int J = 0; J < 4 && Dir != DirEnd; ++J, ++Dir)
----------------
Same as prev. comment, maybe comment on why 4 was chosen here? Maybe use a
named constant?
================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:187
+ StringRef Stem = sys::path::stem(Filename);
+ llvm::SmallVector<StringRef, 2> Dirs; // Only look up the last 2.
+ llvm::StringRef Prefix;
----------------
Maybe add a comment why 2 is chosen here? Also, maybe use a named constant?
Repository:
rC Clang
https://reviews.llvm.org/D45006
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
