[PATCH] D93325: Add srcloc output to clang-query

[Aaron Ballman via Phabricator via cfe-commits]

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