On 11/09/2015 11:53 PM, Trevor Saunders wrote:
I don't really know him, but I don't really disagree with where he wants to get to. However I think we work fairly different ways, and review things differently. When I review patches (mostly for stuff more directly related to Mozilla my standards are basically it needs to be an improvement, and it needs to not introduce bugs.
What I like to ask for is to not just make mechanical changes, but to apply a bit of thought while doing so. Often something better can be done with the same amount of effort (e.g. if one notices that the X_POINTER_IS_Y_POINTER macros are pointless). Such small oddities in the code add up over time, and it is better to eliminate them when making a change anyway.
So I find the it might improve things, but it doesn't also accomplish X to berather odd, and hard to work with if I think getting directly to X might be hard.
In cases where getting to X is hard I'd agree, and the EH_RETURN_HANDLER_RTX thing isn't the best example since it's defined by many ports. What disappointed me in the previous patch series were cases like INITIAL_FRAME_ADDRESS_RTX which is used by only one port. In such a case I don't think it's unreasonable to point out that we actually want to get rid of these macros rather than going through an intermediate stage.
Since EH_RETURN_HANDLER_RTX is a slightly different case, and Joseph said he sees value in the patch, it is OK.
Bernd
