yaxunl added a comment.

In D84362#2274884 <https://reviews.llvm.org/D84362#2274884>, @tra wrote:

> In D84362#2274790 <https://reviews.llvm.org/D84362#2274790>, @yaxunl wrote:
>
>> There are use patterns expecting `PartialDiagnosticInst << X << Y` to 
>> continue to be a `PartialDiagnostic&`, e.g.
>>
>>   PartialDiagnosticAt PDAt(SourceLoc, PartialDiagnosticInst << X << Y);
>>
>> However if we derive PartialDiagnostic and DiagnosticBuilder from a base 
>> class DiagnosticBuilderBase which implements the `<<` operators, 
>> `PartialDiagnosticInst << X << Y` will become a `DiagnosticBuilderBase&`, 
>> then we can no longer write the above code.
>>
>> That's one reason I use templates to implement `<<` operators.
>>
>> Do we want to sacrifice this convenience?
>
> I don't think we have to. 
> AFAICT, virtually all such patterns (and there are only 20-ish of them in 
> total) are used with `EmitFormatDiagnostic(S.PDiag())` which could be adapted 
> to accept `DiagnosticBuilderBase` and `Sema::PDiag()` changed to return 
> `PartialDiagnosticBuilder` with no loss of convenience.

I saw lots of usage of PartialDiagnosticAt, which is defined at

https://github.com/llvm/llvm-project/blob/8c3d0d6a5f5a2a521c4cbae7acbad82a49e2a92f/clang/include/clang/Basic/PartialDiagnostic.h#L417

It requres a PartialDiagnostic as the second argument.

A typical use is like

https://github.com/llvm/llvm-project/blob/69f98311ca42127df92527b6fc3be99841a15f12/clang/lib/Sema/AnalysisBasedWarnings.cpp#L1741

If we use a base class DiagnosticBuilderBase to implement `<<` operators, 
`S.PDiag(diag::warn_unlock_but_no_lock) << Kind << LockName` becomes 
`DiagnosticBuilderBase&` and we can no longer write the above code.

There are ~100 uses like that.


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

Reply via email to