On Mon, Apr 21, 2014 at 5:09 PM, Alp Toker <[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. > FWIW, I'm not particularly a fan of the fluent or builder pattern of interfaces for things like this. If we want to go in that direction, it would seem strange to only do it for this one case. We can change all of the interfaces here whenever we want, so if anything I would go for both (a) reducing the number of them because... wtf? and (b) just keeping them all nice and consistent in the interface they expose. But I'll let Richard comment more on the specifics of this patch. He seemed to prefer the approach as Diego took, and I'm inclined to defer to his judgement here.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
