[PATCH] -include option and -exclude ignore arguments finishing with '/'

[Guillaume Papin]

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:56
@@ +55,3 @@
+  sys::path::const_iterator PathE = sys::path::end(AbsPath);
+  std::vector<StringRef> NewPath;
+  while (PathI != PathE) {
----------------
Why not SmallVector?

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:53
@@ +52,3 @@
+/// path i.e. "..", ".".
+void RemoveRelativePath(SmallString<64>& AbsPath) {
+  sys::path::const_iterator PathI = sys::path::begin(AbsPath);
----------------
Taking a StringRef as argument and returning a string seems more natural to me.

If this is really needed in terms of performance maybe I will use something like http://llvm.org/docs/doxygen/html/Path_8cpp_source.html#l00696 (taking a SmallVectorImpl).


================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:60
@@ +59,3 @@
+      NewPath.pop_back();
+      // If this codition is reached then the given path is incorrect.
+      if (NewPath.size() == 0) {
----------------
codition -> condition

Maybe explain what the condition means instead of the code?

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:61
@@ +60,3 @@
+      // If this codition is reached then the given path is incorrect.
+      if (NewPath.size() == 0) {
+        llvm::errs() << "Incorrect path provided: " << AbsPath << "\n";
----------------
NewPath.empty() maybe?

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:59
@@ +58,3 @@
+    if (PathI->equals("..")) {
+      NewPath.pop_back();
+      // If this codition is reached then the given path is incorrect.
----------------
Shouldn't you check the stack size before doing a pop_back() instead?


================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:95
@@ -70,1 +94,3 @@
+    // Add only non-empty paths to the list.
+    if (Path.size() > 0)
       List.push_back(Path.str());
----------------
Path.empty()

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:62
@@ +61,3 @@
+      if (NewPath.size() == 0) {
+        llvm::errs() << "Incorrect path provided: " << AbsPath << "\n";
+        AbsPath = "";
----------------
Does this error message helps the user?
Maybe if we are sure make_absolute is called beforehand change this to an assert? Unless make_absolute can produce some "wrong" paths.

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:72-73
@@ +71,4 @@
+  AbsPath = "";
+  for (std::vector<StringRef>::iterator I = NewPath.begin(),
+                                      E = NewPath.end();
+       I != E; ++I) {
----------------
Is it Phabricator or the indentation is incorrect here?


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