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

[Jakob Stoklund Olesen]

On Jan 31, 2012, at 2:49 PM, David Greene wrote:

> 
> Add some data structures to represent for loops.  These will be
> referenced during object processing to do any needed iteration and
> instantiation.

- Don't add trailing whitespace.

- Clean up comments after cut/paste (ParseForeachDeclaration).

- Don't put the loop stack in RecordKeeper. It is a parse time data structure.

> struct ForeachLoop {
>   Record *Rec;          // Placeholder for body contents.

MultiClass has 'Record Rec;', no pointer. Why the difference?

>   // Create a second member to hold the list value.  We know that
>   // CurRec is the foreach loop record so this won't conflict with
>   // anything.
>   ItemName = BinOpInit::get(BinOpInit::STRCONCAT, ItemName,
>                             StringInit::get("_List"),
>                             StringRecTy::get())->Fold(CurRec, 0);
>   ForeachListName = ItemName;

Why store the list as a record field? Can't you just store a ListInit directly in ForEachLoop?

- What's DefPrototypes for? I don't see it used anywhere.

- The syntax in the documentation doesn't match the implementation and tests.

- Please avoid putting spurious records in RecordKeeper. It shouldn't be necessary to store unnamed loop and iteration records there.

When resubmitting, please squash everything into one patch.

/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120205/b5950b57/attachment.html>