vsk added a comment.
The testing looks really good. Just a few more requests for documentation
(inline).
================
Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:309
+ /// Return a new SpellingRegion for the SkippedRange if it's valid.
+ Optional<SpellingRegion> adjustSkippedRange(SourceManager &SM,
----------------
This could use some description of what adjustment is done, e.g. "This shrinks
the skipped range if it spans a line that contains a non-comment token. If
shrinking the skipped range would make it empty, this returns None."
================
Comment at: clang/lib/CodeGen/CoverageMappingGen.h:37
+ SourceLocation PrevTokLoc;
+ SourceLocation NextTokLoc;
+
----------------
It'd be helpful to leave inline documentation for these fields as well. It's
clear from context that 'Range' is the skipped source range, but it's not as
clear what {Prev,Next}TokLoc are.
================
Comment at: clang/lib/CodeGen/CoverageMappingGen.h:48
+class CoverageSourceInfo : public PPCallbacks, public CommentHandler {
+ // A pair of SkippedRanges and PrevTokLoc with NextTokLoc
+ std::vector<SkippedRange> SkippedRanges;
----------------
Please end sentences with a '.'
================
Comment at: clang/lib/CodeGen/CoverageMappingGen.h:53
public:
- ArrayRef<SourceRange> getSkippedRanges() const { return SkippedRanges; }
+ // The location of previously token. It's updated when Preprocessor::Lex
+ // lexed a new token.
----------------
How does "Location of the token parsed before HandleComment is called. This is
updated every time Preprocessor::Lex lexes a new token." sound?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83592/new/
https://reviews.llvm.org/D83592
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
