dexonsmith added inline comments.
================
Comment at: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp:247-248
-static const char *reverseOverSpaces(const char *First, const char *Last) {
+static const char *reverseOverSpacesUntilFirstSpace(const char *First,
+ const char *Last) {
assert(First <= Last);
----------------
dexonsmith wrote:
> I don't love this name change, since the idea of the function is to take a
> "first, last" half-open range and reverse "last" back to exclude the spaces.
> "UntilFirstSpace" suggests that it would stop early and make the half-open
> range include a space.
>
> Of course it's all a bit weird because it's not returning a range, just the
> end of it. So the intent of both of these functions is to trim the trailing
> whitespace, it's just this one points at the beginning of the whitespace to
> serve as the end of the range, and the other one points at the last thing
> before the trailing whitespace. Perhaps we can be more explicit about what
> precisely is being returned, instead of describing intent.
>
> What about `findFirstTrailingSpace`?
You might also reimplement this in terms of the other for clarity:
```
static const char *findFirstTrailingSpace(const char *First,
const char *Last) {
const char *LastNonSpace = findLastNonSpace(First, Last);
if (Last == LastNonSpace)
return Last;
assert(isHorizontalWhitespace(LastNonSpace[1]));
return LastNonSpace + 1;
}
```
... unless you think that will hurt performance.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68052/new/
https://reviews.llvm.org/D68052
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
