On 22/04/2014 01:42, Richard Smith wrote:
On Mon, Apr 21, 2014 at 5:09 PM, Alp Toker <[email protected] <mailto:[email protected]>> wrote:


    On 22/04/2014 00:31, Alp Toker wrote:

            I'm not really opposed to the idea, though. I just found
            the current version easier to use.


        I recommend not modifying the Diag() signature because the
        feature will be unavailable to other modules until they each
        update their own Diag() signatures introducing latent churn.


    To be clear, my concern is that other developers will propagate
    this over time to other Diag() functions as they would have to to
    be able to use it -- a problem that a fluent extension to
    DiagnosticBuilder wouldn't suffer from.

    If it does propagate, that means many signatures will get updated
    and locks us in a little more as we look to improve the diagnostic
    system.

    To me that tips the balance in favour of the fluent approach which
    is the straightforward way to provide the feature to all
    diagnostic emitters immediately without further change to interfaces.

    I can tell you're keen to get this in from the 10 minute review
    window but I'd prefer if you take this measure because it's not
    much work beyond your original patch :-)


I think this is a reasonable concern; this is very much a corner-case feature so shouldn't be in the primary interface of Diag(...). I wonder if a manipulator might fit the streaming interface of DiagnosticBuilder a bit better than a fluent interface.

That will work great, thanks.

Alp.


--
http://www.nuanti.com
the browser experts

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to