[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/include/clang/Tooling/Transformer/SourceCode.h:42
+/// terminators. The returned range consists of file locations, if valid file
+/// locations can be found for the associated content; otherwise, and invalid
+/// range is returned.
----------------
s/and/an/


================
Comment at: clang/lib/Tooling/Transformer/SourceCode.cpp:152
+    // will not break anything that removing the entity wouldn't have
+    // already broken.
+  bool TerminatedByMacro = false;
----------------
Indent -2.


================
Comment at: clang/lib/Tooling/Transformer/SourceCode.cpp:156
+  // First, lex to the current token (which is the last token of the range that
+  // we know to be deleted). Then, we process the first token separately from
+  // the rest based on conditions that hold specifically for that first token.
----------------
s/that we know to be deleted/that is being deleted/


================
Comment at: clang/lib/Tooling/Transformer/SourceCode.cpp:164
+  // `TerminatedByMacro`, because we'll want to handle it somewhat differently
+  // for the terminators semicolon and comma. These terminators these can be
+  // safely associated with the entity when they appear after the macro -- extra
----------------
s/These terminators these/These terminators/


================
Comment at: clang/lib/Tooling/Transformer/SourceCode.cpp:282
+                             const LangOptions &LangOpts) {
+  // If the first character is a newline, we'll check for an empty line as a
+  // separator. However, we can't identify an empty line using tokens, so we
----------------
I'm not sure what code implements the "if the first character is a newline" logic. The for loop below checks if LocChars starts with horizontal whitespace followed by vertical whitespace.

Also, I don't see where we find an *empty* line. I understand an empty line as zero or more horizontal whitespace characters between two vertical whitespace characters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72153/new/

https://reviews.llvm.org/D72153