[PATCH] D84362: [NFC] Add missing functions to PartialDiagnostic

Tue Sep 15 08:08:46 PDT 2020

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