[llvm-commits] Case Ranges. New, fresh 'switch' prototype is here! Default case went away.

[Stepan Dyatkovskiy]

Hi Chris,

> 1. IRBuilder.CreateSwitch should take an ArrayRef, not a SmallVector by value.  It needs to work too :)
OK

> 2. Please (as a separate patch) move the bulk of CaseIteratorT out of line into a .cpp file.  I'm not sure how the best way to do this is, maybe using a pImpl or similar trick.
I propose to introduce Impl class, with use-case like this:

type ImplUser::method() {
   Impl(this).doWhatYouWant();
}

We don't keep permanent impl link in this approach. Iterator's fields 
are kept outside the implementation, right in iterator class.

Of course there is also classic way: to keep permanent link to 
implementation, move all iterator fields into the impl, and write top 
level methods like below:
type ImplUser::method() {
   Impl.doWhatYouWant();
}

Which approach do you prefer?

The patch that illustrates the first way is ready, please find it in 
attachment.

> 3. You've got some funky indentation going on (some places using 4 spaces).
OK. I'll fix that.

> Otherwise, the approach is looking fairly reasonable.
Thanks :-)

-Stepan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cr-refactor-2012-10-30-case-it.patch
Type: text/x-patch
Size: 21758 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121030/862e0d0c/attachment.bin>