[PATCH] D68052: [clang-scan-deps] Allow continuation line backslashes followed by whitespace in the dependency source minimizer

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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