[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

Vedant Kumar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 9 12:35:30 PDT 2020


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

I took a closer look at the lexer changes, and think that these look fine. Thanks again for working on this. LGTM with a minor change, described inline.



================
Comment at: clang/lib/Lex/Lexer.cpp:2228
+    if (!NewLinePtr && *CurPtr == '\n')
+      NewLinePtr = CurPtr;
     SawNewline = true;
----------------
It'd be more consistent to write this the way `lastNewLine` is initialized earlier in `SkipWhitespace`, e.g. as:

```
if (*CurPtr == '\n') {
  lastNewLine = CurPtr;
  if (!NewLinePtr)
    NewLinePtr = CurPtr;
}
```

Maybe this could even be factored into a helper lambda, like `setLastNewLine(char *Ptr)` -- I'm not sure whether that'd be cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84988



More information about the cfe-commits mailing list