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@gmail.com> wrote:

On 20 Apr 2017 06:10, "Alexander Barkov" <bar@mariadb.org> 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