[llvm-commits] Case Ranges. New, fresh 'switch' prototype is here! Default case went away.
Stepan Dyatkovskiy
stpworld at narod.ru
Tue Oct 30 06:30:22 PDT 2012
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>
More information about the llvm-commits
mailing list