Let me propose an alternate design for DiagnosticQueue:

• There is no UnderlyingEngine member
• DiagnosticQueue::DiagnosticQueue() just adds a ForwardingDiagnosticConsumer to QueueEngine
• emit() just closes the QueueEngine's transaction, flushing the tentative diagnostics out to consumers

I think that would mean you wouldn't have to modify DiagnosticEngine at all. Does your design have advantages over that one? (It might—I'd like to hear about them.)

So I did play about with ForwardingDiagnosticConsumer a bit, the main issue is that is forwards directly to the consumers and bypasses the target engine altogether, so doesn't update any state on the engine (including hadAnyError!). Now, we should definitely fix that, although it's not clear to me whether all of its clients actually want the diagnostic to be fully re-evaluated by the target engine. There are a couple of other things:

• We'd need to turn a DiagnosticInfo back into a Diagnostic to be re-evaluated by the target engine (as it may need to become a tentative diagnostic). We'd need special handling to make sure e.g warning-as-error is preserved if the original diagnostic engine had it set.
• Because we'd be emitting the diagnostic in a dummy engine, we'd lose the caching for decl rendering (though we probably ought to cache that on the ASTContext or in the evaluator).

I don't think there's anything here that can't be solved, but it seemed less clean to me than creating a dummy diagnostic engine to effectively serve as a Diagnostic builder, which can then be passed onto a target engine. If we want ForwardingDiagnosticConsumer to properly support being able to forward-and-re-evaluate a diagnostic, then I agree we should change this to do that. But either way we'll need DiagnosticEngine changes I believe.