Page MenuHome

Experimental option to use C99 for loop scope for the iterator
Needs ReviewPublic

Authored by Campbell Barton (campbellbarton) on Nov 26 2015, 3:21 AM.

Details

Summary

Similar to the change from rB77ac33db7b449011e3edaa25a24d0ee193b843c1,
but allow to use outer scope, and to define counter type.

This method could be applied to BMesh iterators (BM_ITER_MESH_INDEX) too.

Diff Detail

Repository
rB Blender
Branch
TEMP-GHASH_ITER_INDEX-VARARGS

Event Timeline

Campbell Barton (campbellbarton) retitled this revision from to Experimental option to use C99 for loop scope for the iterator.

Feature-wise I can see the point here, but I’m not convinced we should merge both versions behind a same vararg macro… To me those are actually two different things, would rather have them as two different macros (with different name, maybe GHASH_ITER_INDEX and GHASH_ITER_INDEX_LOCAL?), would find this less confusing…

I don't really see a reason to have separate counter arguments, especially with some "custom" type.

To me it seems more like a size_t field in ghash iter itself and you access it when needed.

@Bastien Montagne (mont29), yep, we could split into 2 macros too.

@Sergey Sharybin (sergey), current iterator access intensionally uses function calls to prevent direct manipulation of the iterator (especially with sets where any direct access is prevented), and often its nice to use short name for counter, replacing array[i] with array[gh_iter.index] is a bit cumbersome.

Own preference is to either, or use multiple macros as @Bastien Montagne (mont29) suggests (though naming is a bit odd).
Or we could leave the code as-is and not attempt to solve this (we somehow managed before C99 made this possible :) ).


Another possibility which allows some more flexibility would be to have give the macro its own scope,

GHASH_ITER_BEGIN(ghash, SomeStruct *key, OtherStruct *value, index) {
   ... foo(key, value, index); ...
} GHASH_ITER_END;

Then the macro can have its own scope before the for loop, assign key, value. Just noting as an alternative way we can extend the macros further, but dont think its really worth doing at this moment.

@Campbell Barton (campbellbarton), having a dedicated parameter which fully describes yet-another-iterator is not less cumbersome than doing gh_iter.index actually. Knowing you, it'll be gh.i ;)

It kinda feels we're trying to do far too much with those macros actually. Using those will also be cumbersome..

Am also wary if trying to be too clever with macros here, unless theres real clear improvement.
Can leave it as-is and abandon this patch.