ymandel added a comment. > For my particular case, I just need multiple notes per rule. I don't need > them to be associated to a particular edit (and in that very particular case, > I don't even need a source location).
I'm not sure I understand: you need this additional note, but it doesn't need to be tied to a source location, so, why can't you concatenate it to the main note? > In terms of the overall design requirements, non-`transformer` checks can > have multiple notes per rule, associated to a source location (and multiple > checks are using the note location to point to somewhere else in the code, so > that is a required feature if we want to be as powerful as the original > clang-tidies, which I think we do). Yes, that's the goal, essentially. And the `Note` field came from exactly that observation. > Notes cannot be associated to a particular `FixitHint`, and I'm not sure > whether that's useful. Right. I'm not sure about that. It seems more intuitive for the note to be bundled with the associated `FixitHint` in the same `DiagnosticBuilder`. That is, if we want to (mostly) go with this design, I would think we'd create a separate diagnostic per note or somesuch. Overall, I'm fine with adding support for additional notes, but I want to get these details sorted out first, since they subtly change the API. > I went with the design in this patch (notes associated to edits) because: > > - it looked like associating a note with an `ASTEdit` was the original > intent, given that `ASTEdit` already had a `Note` field. > - we can plumb notes inside `Metadata`, but `Metadata` is already used for > the warning, so that looks a bit more involved. > > No strong opinion on that front though :) 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
