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

[Ariel Bernal]

================
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);
----------------
Guillaume Papin wrote:
> 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).
> 
I agree, I don't think performance will be an issue in this case. I'll use StringRef.

================
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) {
----------------
Guillaume Papin wrote:
> Why not SmallVector?
done

================
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.
----------------
Guillaume Papin wrote:
> Shouldn't you check the stack size before doing a pop_back() instead?
> 
yes, I got this one in the new patch

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:62
@@ +61,3 @@
+      if (NewPath.size() == 0) {
+        llvm::errs() << "Incorrect path provided: " << AbsPath << "\n";
+        AbsPath = "";
----------------
Guillaume Papin wrote:
> 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.
we can't rely on make_absolute since the user can specify ../../.. till the root is reached. We could silently return an empty path and remove it from the list but I think it's better if at least we provide some feedback.

================
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) {
----------------
Guillaume Papin wrote:
> Is it Phabricator or the indentation is incorrect here?
already fixed

================
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());
----------------
Guillaume Papin wrote:
> Path.empty()
done


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