[PATCH] D143906: [include-cleaner] Better support ambiguous std symbols

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks!



================
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:85
 
+Hints isPublicHeader(const FileEntry *FE, const PragmaIncludes *PI) {
+  return (PI->isPrivate(FE) || !PI->isSelfContained(FE)) ? Hints::None
----------------
we actually need to nullcheck for PI here, defaulting to PublicHeader when it isn't present.


================
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:86
+Hints isPublicHeader(const FileEntry *FE, const PragmaIncludes *PI) {
+  return (PI->isPrivate(FE) || !PI->isSelfContained(FE)) ? Hints::None
+                                                         : Hints::PublicHeader;
----------------
nit: maybe unwrap this? e.g:
```
if (PI->isPrivate(..) ... ) return Hints::None;
return Hints::PublicHeader;
```


================
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:95
+    Results.emplace_back(H, Hints::PublicHeader);
+    for (const auto *Export : PI->getExporters(H, SM.getFileManager()))
+      Results.emplace_back(Header(Export), isPublicHeader(Export, PI));
----------------
we need to nullcheck for `PI` here


================
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:134
+  }
+  return hintedHeadersForStdHeaders(Headers, SM, PI);
+}
----------------
can you also wrap this in `applyHints(..., Hints::CompleteSymbol)`, similar to what `locateSymbol` does for rest of the stdlib symbols


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143906