aaron.ballman added a comment.
Thank you for this, it looks really useful!
================
Comment at: clang-tools-extra/clang-query/Query.cpp:94
+void dumpLocations(llvm::raw_ostream &OS, ast_type_traits::DynTypedNode node,
+ ASTContext &Ctx, const DiagnosticsEngine &Diags,
----------------
================
Comment at: clang-tools-extra/clang-query/Query.cpp:97
+ SourceManager const &SM) {
+ auto locs = clang::tooling::NodeLocationIntrospection::GetLocations(node);
+
----------------
It should be named `Locs` per the usual coding conventions.
(I'll hold my nose on the use of `auto` here, but if it got spelled out since
you're touching the line anyway, that would not be a terrible thing.)
================
Comment at: clang-tools-extra/clang-query/Query.cpp:99
+
+ for (auto it = locs.LocationAccessors.begin();
+ it != locs.LocationAccessors.end(); ++it) {
----------------
`it` -> `Iter`
================
Comment at: clang-tools-extra/clang-query/Query.cpp:106
+ TD.emitDiagnostic(FullSourceLoc(it->first, SM), DiagnosticsEngine::Note,
+ "SourceLocations here", None, None);
+
----------------
`SourceLocations` -> `source locations` ?
================
Comment at: clang-tools-extra/clang-query/Query.cpp:108
+
+ auto commonLoc = it->first;
+ auto scout = it;
----------------
`CommonLoc` and the type should definitely be spelled out.
================
Comment at: clang-tools-extra/clang-query/Query.cpp:109
+ auto commonLoc = it->first;
+ auto scout = it;
+ while (scout->first == commonLoc) {
----------------
`Scout`
================
Comment at: clang-tools-extra/clang-query/Query.cpp:123
+
+ for (auto it = locs.RangeAccessors.begin(); it != locs.RangeAccessors.end();
+ ++it) {
----------------
`it` -> `Iter`
================
Comment at: clang-tools-extra/clang-query/Query.cpp:136
+ DiagnosticsEngine::Note,
+ "SourceRanges here " + it->first.printToString(SM),
+ CharSourceRange::getTokenRange(it->first), None);
----------------
`SourceRanges` -> `source ranges` ?
================
Comment at: clang-tools-extra/clang-query/Query.cpp:139-140
+
+ auto commonLoc = it->first;
+ auto scout = it;
+ while (scout->first == commonLoc) {
----------------
Same here as above.
Given how common this code seems to be with the first block of code, would it
make sense to turn some of this code into a lambda?
================
Comment at: clang-tools-extra/clang-query/Query.cpp:152
+ }
+ for (auto it = locs.RangeAccessors.begin(); it != locs.RangeAccessors.end();
+ ++it) {
----------------
`it` -> `Iter`
================
Comment at: clang-tools-extra/clang-query/Query.cpp:165
+ FullSourceLoc(it->first.getBegin(), SM), DiagnosticsEngine::Note,
+ "SourceRange " + it->first.printToString(SM) + " starting here...",
+ CharSourceRange::getTokenRange(it->first), None);
----------------
`SourceRange` -> `source range` ?
================
Comment at: clang-tools-extra/clang-query/Query.cpp:168-169
+
+ auto colNum = SM.getPresumedColumnNumber(it->first.getEnd());
+ auto lastLineLoc = it->first.getEnd().getLocWithOffset(-(colNum - 1));
+
----------------
More names to correct for conventions. Can you also spell out at least the
location datatype?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93325/new/
https://reviews.llvm.org/D93325
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
