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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
