hokein added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:219
+ if ((Top.Block && HashLine > Top.SeenAtLine) ||
+ Top.SeenAtLine == HashLine) {
+ Out->ShouldKeep.insert(HashLine);
----------------
nit: remove the brace for the `if`, the same below.
================
Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:349
EXPECT_TRUE(PI.shouldKeep(3));
+ EXPECT_FALSE(PI.shouldKeep(13));
+ EXPECT_TRUE(PI.shouldKeep(14));
----------------
with more tests being added, these line-number-inlined tests now are hard to
follow, even with the line comment in the example.
I think we can polish these tests by using the annotation point `^`, something
like.
```
llvm::Annotation Code = R"cpp(
$keep1^#include "keep1.h" // IWYU pragma: keep
$normal^%include "normal.h"
)cpp";
EXPECT_TRUE(PI.shouldKeep(offsetToLine(Code.point("keep1"))));
EXPECT_FALSE(PI.shouldKeep(offsetToLine(Code.point("normal"))));
```
We need to implement an `offsetToLine` function, it should be trivial to add
(basically calculate the number of `\n`s to the offset point).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138797/new/
https://reviews.llvm.org/D138797
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
