aaron.ballman added a reviewer: rsmith. aaron.ballman added a subscriber: rsmith. aaron.ballman added a comment.
In D84362#2271585 <https://reviews.llvm.org/D84362#2271585>, @tra wrote: > So, the idea here is to do some sort of duck-typing and allow DiagBuilder to > work with both `DiagnosticBuilder` and `PartialDiagnostic`. > > What bothers me is that unlike `Diagnostic` `PartialDiagnostic` seems to be > commingling functionality of the builder with that of the diagnostic itself. > I'm not sure if that's by design or just happened to be that way. > > I think a better approach would be to refactor `PartialDiagnostic` and > separate the builder functionality. That should make it possible to create a > common diagnostic builder base class with Partial/Full diagnostic deriving > their own builder, if needed. > > That said, I'm not that familiar with the diags. Perhaps @rtrieu > @aaron.ballman would have better ideas. I'm similarly a bit uncomfortable with adding the SFINAE magic to make this work instead of making a base class that will work for either kind of diagnostic builder. I'm adding @rsmith to see if he has opinions as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84362/new/ https://reviews.llvm.org/D84362 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
