[Bf-committers] Not adding #defined code in sources please

[Campbell Barton]

On Fri, Jan 25, 2013 at 11:31 PM, Ton Roosendaal <ton at blender.org> wrote:
> Hi,
>
> As part of our style guide, I propose to limit 'fake inlining' with #defines of code to an absolute minumum.
> It makes debugging (breakpoints, code stepping) impossible, and makes code less readable.
>
> This is totally unnecessary for example:
>
> http://projects.blender.org/scm/viewvc.php/trunk/blender/source/blender/editors/sculpt_paint/paint_vertex.c?root=bf-blender&r1=53285&r2=53321

Would you suggest this instead as being an improvement?

http://www.graphicall.org/ftp/ideasman42/paintweight_use_static_func_diff.txt

There are quite a few ugly defines in blenders code, try grep for...

PLUGIN_DECLARE_SERIAL, ShowDeprecationWarning, BRICONTRGB,
COLRAMP_GETPATH, BRUSH_CAPABILITY, DRAW_EM_MEASURE_STATS_FACEAREA,
KEYFRAME_OK_CHECKS, HIGHLIGHT, ARRAY_SWAP, BLI_STR_UTF8_CPY,
PUSH_HEAP_BODY, POP_HEAP_BODY, PASSATTRIB, SINGLETARGETNS_GET_TARS....
more but you get the point.

Would you say these are bad too?

If so, should we do code cleanups to make out code less bad? :). Or do
we invoke the "this is reliable production code rule" - even though
the code is horrible to debug.


There are factors which are tedious to discuss in mail...
* Weather the macro makes calls to functions.
* If the macro is likely to need debugging. VECCOPY is example where
its not an issue so much.
* If the macro makes use of ##name## in a way which would make not
using a macro much more tricky...
* If the macro ends up needing to have the entire namespace of the C
function passed along to perform some simple operation as is the case
with paint_vertex.c patch linked above.

Ton - I strongly agree its good to limit use of macros as functions -
but  there are exceptions to the rule... What is/isn't OK by your
definition I'm still not sure on.

I understand if you don't like long discussions about these topics
(its really tedious!), but if you say my code is doing-it-wrong. I
like to point out there are quite a few factors that lead to me making
this choice and I think it was a reasonable one.

Of course we can just put up with passing many variables around to
static functions, or copying and pasting code about rather then using
defines, its becomes a matter of personal preference at some point
too.

> Also defines like "LIKELY" or "UNLIKELY" should go away - unless in local code segments that require heavily optimizing.
>
> Adding inline functions should also be limited to cases where it's proven to be required and giving massive speedups. Optimizing in general shouldn't be needed ever. Good code design is always superior.
>
> So let's stop attempting to do code clean work now, unless approved in advance by the people who actively work on that code.
>
> Laters,
>
> -Ton-
>
> ------------------------------------------------------------------------
> Ton Roosendaal  Blender Foundation   ton at blender.org    www.blender.org
> Blender Institute   Entrepotdok 57A  1018AD Amsterdam   The Netherlands
>
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers



-- 
- Campbell