VitaNuo added inline comments.
================
Comment at:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:63
+ bool shouldKeep(unsigned HashLineNumber) const;
+ bool shouldKeep(const FileEntry *FE) const;
----------------
Why wouldn't you actually inline the implementation for both these functions?
The implementations seem trivial enough.
================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:37
#include <utility>
+#include <vector>
----------------
I don't think you've added a dependency on all these headers in this patch.
Nice if you've actually fixed the missing include violations all over the file,
but please double-check that there are no debugging etc. artefacts left.
================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:272
+ FileID CommentFID = SM.getFileID(Range.getBegin());
+ auto *FE = SM.getFileEntryForID(CommentFID);
----------------
You might want to inline this call, it seems to have a single usage right below.
================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:288
+ if (Pragma->consume_front("always_keep")) {
+ Out->AlwaysKeep.insert(FE->getUniqueID());
+ return false;
----------------
I believe you need to check `FE` for `nullptr`, similar to private pragma
handling above.
Also, the code above does `FE->getLastRef().getUniqueID()`, is there any
difference to `FE->getUniqueID()`? I wouldn't expect so, but then you could
maybe unify this one with the above, this way or the other.
================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:421
+ return AlwaysKeep.contains(FE->getUniqueID());
+}
+
----------------
As mentioned above, consider inlining.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156122/new/
https://reviews.llvm.org/D156122
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
