djasper added inline comments. ================ Comment at: include/clang/Tooling/Core/Replacement.h:228 @@ -226,3 +227,3 @@ /// \pre Replacements must be for the same file. -std::vector<tooling::Range> -calculateChangedRangesInFile(const tooling::Replacements &Replaces); +std::vector<Range> calculateChangedRangesInFile(const Replacements &Replaces); + ---------------- This was pre-existing, but I'd remove the "InFile" suffix. It doesn't seem to add any information.
================ Comment at: include/clang/Tooling/Core/Replacement.h:230 @@ +229,3 @@ + +typedef std::map<const std::string, Replacements> + FileToReplacementsMap; ---------------- I think the key type in a map is always const, so no need for "const". ================ Comment at: include/clang/Tooling/Core/Replacement.h:235 @@ -229,1 +234,3 @@ +/// related to the same file entry are put into the same vector. +FileToReplacementsMap groupReplacementsByFile(const Replacements &Replaces); ---------------- I also wonder whether this should really return a map. I find such maps in interfaces sometimes a bit difficult as they have some cost and the subsequent analyses might actually prefer a different container. How about instead just returning a vector<Replacements>? I mean the Replacements themselves already have the filename stored in them so the key in the map is redundant anyway. Manuel, any thoughts? http://reviews.llvm.org/D17852 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
