RFC; Compiler warnings / Addresses from temporary

[nicolas]

First of all, hello everyone.

My first post here is a proposal.

We are presently using IPlug+WDL with the GCC compiler collection on OSX, windows and linux and we are encountering many reports of GCC such as:


/Users/nicolas/Code/ln2/third-party/cockos.com/wdl/IPlug/IGraphicsCocoa.mm: In function 'void -[IGraphicsCocoa_v1002 drawRect:](IGraphicsCocoa_v1002*, objc_selector*, NSRect)':
/Users/nicolas/Code/ln2/third-party/cockos.com/wdl/IPlug/IGraphicsCocoa.mm:81: warning: taking address of temporary
/Users/nicolas/Code/ln2/third-party/cockos.com/wdl/IPlug/IGraphicsCocoa.mm: In function 'void -[IGraphicsCocoa_v1002 mouseDown:](IGraphicsCocoa_v1002*, objc_selector*, NSEvent*)':
/Users/nicolas/Code/ln2/third-party/cockos.com/wdl/IPlug/IGraphicsCocoa.mm:104: warning: taking address of temporary
/Users/nicolas/Code/ln2/third-party/cockos.com/wdl/IPlug/IGraphicsCocoa.mm:107: warning: taking address of temporary


This happens in many places within IPlug because the following coding style is used:


void g(IRECT* rect, ...);

void f()
{
...
g(&IRECT(0, 0, w, h), ...);
}


Our proposal is to switch to the following style instead:


void g(IRECT const& rect, ...);

void f()
{
g(IRECT(0, 0, w, h), ...);
}


It saves a character per usage, removes a warning, guarantees that the reference is used in a constant context, and should behave exactly the same in terms of performance.

It does, however, requires to change the interface of controls (etc) thus requiring a bit of rewriting for everyone, to remove all those extra & characters.

We can provide a patch against the WDL if desired, and would rather prefer not have to have our own private fork of WDL for such a small thing.

[cc_]

I would love to not have those warnings so I would gladly change my code to handle the new interface. Or use your patch if it didn't make it into the main code base.

Have you checked it works? I mean it is possible that some of those objects are not treated as const inside the functions.

[nicolas]

For now I've done my patch for OSX only.

IIRC the only case where I could not define the reference const was for IGraphics::isDirty, which intends to modify the IRECT passed to it.

It's easily solved by using a non constant reference (or keeping usage of a pointer)

As a pure C coder I am not that fond of references usually, but for passing a value from the stack that is meant to be copied somewhere else, they work nicely. Basically anywhere where we want close to "pass by value" semantics.

[dub3000]

would it be possible to overload all those calls with clean ones? the overloads would just call the original ones. you could probably localize all that in one spot and then you wouldn't need to fork it, just maintain one extra header file.

e.g.

// original
void g(IRECT* rect, ...);

// overloaded
void g(IRECT const& rect, ...) {
g(&rect, ...); // would this error out?
}

void f()
{
g(IRECT(0, 0, w, h), ...);
}

[schwa]

Although this is exactly the case that pass-by-reference is designed for, there's no other pass-by-reference in iplug, so my personal vote would be to keep the stylistic consistency and just be explicit:


IRECT r(0,0,w,h);
f(&r);


There should be no performance difference. The stylistic consistency is helpful to keep the code accessible, not just for cosmetic reasons.

[nicolas]

Indeed the explicit way was at first more appealing to me for the same reasons.

It however is:

1- longer-winded.
2- as a user of those functions I still am always wondering whether I can rightfully do that, since I'm left to wonder if my reference might escape the scope of the calling function. So each time I call such a function/method, I'm tempted to look at its implementation to make sure copying is taking place.

No such cognitive dissonance happens with the by-reference call, since it looks like a pass-by-value.

Edit:
By the way, for the IControl constructors, I believe passing the IRECT by value (thus copying it) wouldn't be such a performance issue anyway. First because it's not something you do in an inner loop, second because the functions are so simple I would actually expect a compiler to perform that optimization on its own.
dub3000: doubling each method with an overload that converts from reference to pointers would really not be such good fun for maintenance. Plus it would only help for user code.

[cc_]

I use the explicit version mostly in my code.

But how would it handle cases like this constructor in IControl.h?


IKnobRotaterControl(IPlugBase* pPlug, int x, int y, int paramIdx, IBitmap* pBitmap,
double minAngle = -0.75 * PI, double maxAngle = 0.75 * PI, int yOffsetZeroDeg = 0,
EDirection direction = kVertical, double gearing = DEFAULT_GEARING)
: IKnobControl(pPlug, &IRECT(x, y, pBitmap), paramIdx, direction, gearing),
mBitmap(*pBitmap), mMinAngle(minAngle), mMaxAngle(maxAngle), mYOffset(yOffsetZeroDeg) {}