hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
It looks good to me.
As discussed offline, would be nice to show line number for each line of code
in the html dump.
================
Comment at: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp:90
+ std::vector<Target> Targets;
+ std::vector<std::pair</*Offset*/ unsigned, /*TargetIndex*/ unsigned>> Refs;
+
----------------
This is for main-file right? worth a comment.
================
Comment at: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp:97
+ void addRef(SourceLocation Loc, const NamedDecl &D) {
+ auto Coords = SM.getDecomposedLoc(SM.getFileLoc(Loc));
+ if (Coords.first != File)
----------------
using `auto [RefFileID, Offset] = SM.getDecomposedLoc(SM.getFileLoc(Loc));` is
more readable, comparing the following `.first`, `.second` usage.
================
Comment at: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp:101
+ Targets.push_back({&D});
+ Refs.push_back({Coords.second, Targets.size() - 1});
+ }
----------------
If I read this code correctly, `Refs` should only collect main-file refs, if so
then we should bailout if `Coords.first != File`.
================
Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:1
+#include "clang-include-cleaner/Record.h"
+#include "clang/Frontend/FrontendAction.h"
----------------
nit: missing a file comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135956/new/
https://reviews.llvm.org/D135956
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
