[PATCH] D143906: [include-cleaner] Better support ambiguous std symbols
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 14 00:39:44 PST 2023
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
More information about the cfe-commits
mailing list