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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits