[PATCH] D138797: [include-cleaner] Implement IWYU begin_keep/end_keep pragma support.

[Haojian Wu via Phabricator via cfe-commits]

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