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