courbet added a comment.
Thanks for the comments.
================
Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:250-251
+// Adds a note to the given edit or edits. If there are several edits, the note
+// is added to each one of them.
+// \code
----------------
ymandel wrote:
> li.zhe.hua wrote:
> > Are we sure that this is what someone would want? Perhaps I am not creative
> > enough, but this seems like it could explode a large number of notes that
> > coincide to edits, which seems like an odd thing to want.
> Agreed. I'm more inclined to only include the ASTEdit version. I see the
> value of the other one, but seems easy to get wrong. Alternatively, implement
> it to either only add to the first edit or append/prepend a separate edit
> with the note.
>
> Either way, I think the primary use should be with a standalone note
> generator like `ASTEdit note(TextGenerator Note)`. The idea is that the new
> combinator allows adding notes to a rule and we happen to store notes in
> ASTEdits. Then, `withNote` is really the uncommon case where you want to
> supplement an existing edit or set of edits with a note. WDYT?
> I'm more inclined to only include the ASTEdit version. I see the value of
> the other one, but seems easy to get wrong. Alternatively, implement it to
> either only add to the first edit or append/prepend a separate edit with the
> note.
The `EditGenerator` version was so that we can always add a note. Some
generators return an `EditGenerator` instead of an `ASTEdit`. For example,
`noopEdit` needs access to the match result to generate an empty range . The
second version is required to work with these:
```
withNote(changeTo(anchor1, ...)); // OK, changeTo returns an `ASTEdit`.
withNote(noopEdit(...)); // not OK, noopEdit returns an `EditGenerator`, we
need the second version.
```
> Either way, I think the primary use should be with a standalone note
> generator like ASTEdit note(TextGenerator Note). The idea is that the new
> combinator allows adding notes to a rule and we happen to store notes in
> ASTEdits. Then, withNote is really the uncommon case where you want to
> supplement an existing edit or set of edits with a note. WDYT?
I think you're right, we don't need the notes to be attached to a particular
edit. I expect most notes to be standalone: that's actually what I need in my
case, and it's also actually what the `diag` API presents to the user.
```
diag(loc1, "some warning") << FixItHint::CreateReplacement(...);
diag(loc2, "some note", clang::DiagnosticIDs::Note)
```
becomes, in transformerland:
```
makeRule(matcher, flatten(changeTo(...), note(anchor2, cat("some note"))),
cat("some warning"))
```
So I think we only really need an ` ASTEdit note()` generator. I went with that
solution.
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
