Re: [Libreoffice] [PATCH] improve style according to cppcheck

On Sat, 2010-11-27 at 13:39 -0600, Norbert Thiebaud wrote:
> On Sat, Nov 27, 2010 at 12:09 PM, Christian Lohmaier
> <lohmaier+libreoffice@googlemail.com> wrote:
> > Hi *,
> >
> > On Sat, Nov 27, 2010 at 5:40 PM, Takeshi Abe <tabe@fixedpoint.jp> wrote:
> > > [cppcheck patches]
> >
> > I'm curious: Why does cppcheck "complain" about "for.... i++" and
> > suggests/demands pre-increment instead (for ... ++i)?
> >
> > Is there any noticable difference?
>
> In my opinion:
> If there is a noticeable difference -- in a place where the 2 forms
> are indeed interchangeable -- then this is a bug to be filed with the
> compiler.
> And if they are not interchangeable in the context, then cppcheck
> should be silent....

Just in case anyone was wondering, this is about stl iterators not plain
ints or floats. Compilers do typically have no problem with plain old
types. I'm a fan of saying what you mean nevertheless even in those
cases :-)

As Takeshi pointed out in another mail, the postfix form returns a copy
of the iterator as it was before the increment, while the prefix form
returns a reference to the current value after its increment. So postfix
requires an extra copy which makes it inherently slower.

I'm not so sure that compilers can optimize STL postfix operators with
the ease they do for basic types. Their implementation differs quite a
bit from one impl to another. I expect that the full impl would need to
be visible to the compiler for it to have a chance to do the right
thing. Typically that's probably the case and the full template is
available to the compiler in each compilation unit in order to elide out
the code patchs that generate the old copy which the postfix form
returns, so they *may* be able to do it. But you're at the mercy of what
the compiler can do at that stage, the compiler may be forced to e.g.
stop inlining and optimize for size, and so make function calls, and at
that stage they nay merge all the postfix++ calls together, with the
corresponding performance loss in the cases where the temporary isn't
needed.

Anyway, I reckon its worth tackling the cppcheck warnings about the
postfix operator being used where a prefix would suffice.

C.