[PATCH] Fixes a bug when iterating on paths

Rafael Ávila de Espíndola rafael.espindola at gmail.com
Wed Aug 7 21:31:45 PDT 2013


  LGTM with small fixes.


================
Comment at: unittests/Support/Path.cpp:150
@@ +149,3 @@
+
+  StringRef(Path).split(ExpectedPathComponents, "/", -1, false);
+
----------------
Nit: why do you need to pass -1 and false?


================
Comment at: unittests/Support/Path.cpp:173
@@ +172,3 @@
+  // The root path will also be a component when iterating
+  ExpectedPathComponents.insert(ExpectedPathComponents.begin(), "/");
+
----------------
Might be cleaner to not pass -1 and false to split and then do

ExpectedPathComponent[0] = "/";


================
Comment at: unittests/Support/Path.cpp:198
@@ +197,3 @@
+  // when iterating.
+  ExpectedPathComponents.insert(ExpectedPathComponents.begin()+1, "\\");
+
----------------
Same as above.

================
Comment at: lib/Support/Path.cpp:80
@@ -79,3 +79,3 @@
     // * {file,directory}name
-    size_t end = path.find_first_of(separators, 2);
+    size_t end = path.find_first_of(separators, 0);
     return path.substr(0, end);
----------------
0 is the default, please drop it 


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

BRANCH
  svn

ARCANIST PROJECT
  llvm



More information about the llvm-commits mailing list