[PATCH] D95202: ADT: Use 'using' to inherit assign and append in SmallString

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good - some optional feedback on the code fixes. Please run clang-format on the changes here (phab lint picked up some cases in the test code that could be cleaned up).



================
Comment at: clang/lib/Frontend/ModuleDependencyCollector.cpp:196
+    if (DroppedDotSlash.begin() != AbsoluteSrc.begin())
+      AbsoluteSrc.erase(AbsoluteSrc.begin(), DroppedDotSlash.begin());
+  }
----------------
Maybe just call this inconditionally - it's probably cheap enough when the range is zero-length?

Perhaps it'd be more readable if "remove_leading_dotslash" had a version that could return the prefix length to remove.

But also, this is ultimately passed to 'getRealPath' which takes a StringRef, so there's technically no need to rewrite this SmallString - the StringRef from remove_leading_dotslash could be passed instead, something like:
```
StringRef TrimmedAbsoluteSrc = path::remove_leading_dotslash(AbsoluteSrc);
...
if (!getRealPath(AbsoluteSrc, CopyFrom))
...
```


================
Comment at: llvm/lib/Support/FileCollector.cpp:89-93
+  {
+    StringRef DroppedDotSlash = sys::path::remove_leading_dotslash(AbsoluteSrc);
+    if (DroppedDotSlash.begin() != AbsoluteSrc.begin())
+      AbsoluteSrc.erase(AbsoluteSrc.begin(), DroppedDotSlash.begin());
+  }
----------------
Similar comment here as with ModuleDependencyCollector (actually there's a bunch of similar code here - perhaps it could be deduplicated in some way)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95202/new/

https://reviews.llvm.org/D95202