[llvm-commits] [llvm] r134907 - /llvm/trunk/utils/TableGen/

[Chris Lattner]

On Jul 13, 2011, at 9:33 AM, John Criswell wrote:

> On 7/12/11 11:15 PM, Chris Lattner wrote:
>> [snip]
>> I understand, but the policy of the project is not based around the submitter's schedule.  While I imagine that it is *extra* frustrating to hear this, the policies of the project are aimed at making sure the *right code* goes in, not in *reducing barriers* to submission.  Having a high quality bar is a good thing.
>> 
>> That said, not getting prompt review is a bug, not a feature.  The reality is that we're all human, and all busy.  Part of working successfully with the project is finding ways that increase the odds that someone will say "go ahead and commit, it seems obvious".  Separating trivial and mechanical changes from the "interesting" ones is a great way to do that.  Making the "interesting" changes tiny is another great way.
> 
> What is your recommendation for something like the SAFECode patch?  The SAFECode patch adds several new transforms and a run-time library to LLVM.  It is large merely because it adds new code.  The transform passes are pretty simple and are easier to review individually, so I could split them into separate patches.  However, the transforms are useless individually; they have to be used in conjunction to do anything meaningful.
> 
> Would you recommend splitting the patch into several smaller ones in which each patch adds one of the transforms?  Or should the patch stay as a whole for review?

Adding major new functionality like this is tricky on two levels:

1. You need to get general buy in that the new feature and functionality is a good thing.  This can be hard if it is something radically new. :)

2. You need someone to review the patches.  This is hard because it is generally a bunch of code.

I know I promised I'd try to look at it last weekend, unfortunately I'm still dealing with thrashing the type system to death and other exciting stuff.  I'm hopefully I'll get to it in the next few days.

-Chris