[PATCH] D128807: [clang][transformer] Finish plumbing `Note` all the way to the output.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D128807#3622727 <https://reviews.llvm.org/D128807#3622727>, @li.zhe.hua wrote:

> - A note being a part of an edit seems weird at best. An `ASTEdit` and `Edit` are fragments of a greater, logical change. That a note should semantically be associated with the insertion of an open paren "`(`" but not the close paren "`)`" seems odd to me.
> - The note takes the location of the edit it is attached to. Perhaps that could be of some convenience when those coincide, but I don't believe that should necessarily be the case. I'm imagining notes could be used to point out //other// parts of the source that are interesting.

Eric, these are great points. Originally, the idea for note (IIRC) came from the observation that sometimes a single match will generate edits in multiple locations, each which deserve a different diagnostic note (perhaps the same text, but appearing at the respective location). The intent was *not* to allow noting the insertion of paren, for example.  So, I think it was a mistake. Yes, sometimes an ASTEdit is a coherent change, but sometimes (as the paren example demonstrates) its just a fragment.

So, from my original intent, I think that we'd just want to support multiple notes per rule, with each keyed on a source range.  That said, we could decide to use `ASTEdit` for that purpose, assuming we're ok with `ASTEdit` with a null `Replacement` field. But, we'd have to think about th implications before we go down that route.

Clement -- what was your intended application? That may help clarify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128807