[llvm-commits] [PATCH 01/13] Add For Loop Structures

[Jakob Stoklund Olesen]

On Feb 7, 2012, at 10:08 AM, "David A. Greene" <dag at cray.com> wrote:

> Jakob Stoklund Olesen <stoklund at 2pi.dk> writes:
> 
>> The roundtrip delay on pre-commit reviews makes it necessary to review
>> larger changes than one would normally commit directly. 
> 
> Ah.  I'm assuming I should submit the patch as review, not break
> it up.  Is that right?

Yes. I don't think it makes sense to review a series of commit-sized changes. A single patch can be quite large as long as it is doing just one thing, see for example Hal's vectorizer.

>> In the present case, I think the granularity you chose is fine: 1. Add
>> a single-variable foreach loop. 2. Zip.
> 
> Ok.  I thought zip was a no-go.  I'm going to wait on it anyway while I
> work out how various things might work.  It may or may not come back.

Let's discuss the design before you put too much work into it.  The example you pasted looked like it would require a laundry list of syntax and type system extensions.  I don't want to do that for something that probably won't be used much.

Back in October, we discussed something much simpler: "foreach x = […], y = […] in …". That's a fairly simple extension to your first patch, and it doesn't require you to invent tuple types and generic zip operators. I think that would be fine to add.

/jakob