← Back to team overview

maria-developers team mailing list archive

Re: Refactoring in LEX_STRING and LEX_CSTRING

 

Hi Monty,

I think that we should follow the modern conventions here.
The Google style guide (which the MySQL group at Oracle is trying to
follow) says that it is OK to pass references to const objects, but it
generally forbids references to non-const:
https://google.github.io/styleguide/cppguide.html#Reference_Arguments

It also gives examples on when pointers to const could be preferred over
references to const.

Like Bar noted, it is a well-known fact that using const references allows
some compiler optimizations. Here is some discussion:
http://stackoverflow.com/questions/13318257/const-reference-to-temporary-vs-return-value-optimization

On a related note, I hope that 10.3 can switch the language dialect to
C++11, and that we can have a session on modern C++ in the next developer
meeting.

I would like to contribute some detailed comments below.

On Thu, Apr 20, 2017 at 8:35 AM, Michael Widenius <
michael.widenius@xxxxxxxxx> wrote:

> On 20 Apr 2017 06:10, "Alexander Barkov" <bar@xxxxxxxxxxx> wrote:
>
> With the "passing by pointer" approach whenever one needs to pass
> a LEX_STRING to a function accepting LEX_CSTRING, one will have to do:
>
>
> void func1(const LEX_STRING *str)
> {
>   LEX_CSTRING tmp= {str->str, str->length};
>   func2(&tmp);
> }
>
> The above is not a problem:
>
>
> - We don't have less than 5 places in the code where LEX_STRING is changed
> to LEX_CSTRING.
> - In these places one can do this with a simple cast, nothing elaborated.
>

My preference for cases like this would be an inline wrapper function, even
with the same name as the wrapped function (different call signature).
Simple inline functions do not bloat the generated code (provided that they
do not do any stupid things such as copying large objects), and they also
keep the source code tidier.

With wrapper functions, the number of type casts can be minimized. Type
casts are inherently dangerous, especially the C-style casts. By the way,
C++11 introduces the typename{} cast where information cannot be lost,
e.g., long a{x} is fine if x can be converted to long without loss, but
throws an error if not (say, sizeof(x) > sizeof(a)).'

For the "but C is more efficient than C++" crowd, I would recommend
watching the CppCon 2016 presentation "Rich code for tiny machines"
https://www.youtube.com/watch?v=zBkNBP00wJE and looking at the code
generated from
https://github.com/lefticus/x86-to-6502/blob/master/examples/pong.cpp

Also, you can have a look at the puzzle solvers
http://www.iki.fi/~msmakela/software/pentomino/ that I recently converted
from C to modern C++. To my surprise, the C++ version compiled by clang -O3
was slightly faster than the C version compiled by GCC. So, lightweight
objects do not necessarily add any overhead. While I was doing the
conversion to C++, I fixed a bug in the symmetry reduction of the 3D puzzle
solver. This demonstrates that breaking the problem into more manageable
pieces (abstract data types) can improve code quality. If it is done
properly, any run-time overhead can be avoided.

we use "const Lex_cstring &str" all around C++ code.
>
>
> 2. Instead of "const LEX_CSTRING *str"
> No, no, no.
>
> I do not want to use & anywhere as arguments in the code. It hides things
> and lends people to do errors like we had before where they declared some
> functions to take full structure she references in other places, which
> creates a mess.
>

The Google coding style generally agrees with you, by forbidding references
to non-const objects.

I cannot see any harm in passing references to const. (Of course, the code
must be well-written: the passed object must not be modified, not even via
anything that casts the constness away and then modifies the object.
const_cast is only OK to use when the object is truly not modified, and
C-style casts should be avoided altogether in C++ code.)

Best regards,

Marko

References