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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
