[PATCH] D138678: [include-cleaner] Capture private headers in PragmaIncludes.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:237
+      auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin()));
+      if (!FE) {
+        return false;
----------------
nit: you can drop the braces here. LLVM convention is to not have braces when a condition/loop body has only a single statement.


================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:240
+      }
+      if (Pragma->consume_front(", include ")) {
+        // We always insert using the spelling from the pragma.
----------------
nit: you can also rewrite this as:
```
StringRef PublicHeader;
if (Pragma->consume_front(", include ")) {
  PublicHeader = save(Pragma->startswith("<") || Pragma->startswith("\"")
                      ? (*Pragma)
                      : ("\"" + *Pragma + "\"").str());
}
Out->IWYUPublic.insert({FE->getLastRef().getUniqueID(), PublicHeader});
```


================
Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:381
+    // IWYU pragma: private
+    class Private {};
+  )cpp";
----------------
VitaNuo wrote:
> kadircet wrote:
> > nit: no need for the declaration of `Private` here (nor in `private.h`). it's actually introducing a double definition error into TU now.
> Out of curiosity: what's TU?
it means `translation unit`, it's the source file provided to the compiler after all preprocessor directives are processed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138678