maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hi Monty,


On 04/20/2017 09:35 AM, Michael Widenius wrote:
> Hi! 
> 
> On 20 Apr 2017 06:10, "Alexander Barkov" <bar@xxxxxxxxxxx
> <mailto:bar@xxxxxxxxxxx>> wrote:
> 
>     Hello all,
> 
> 
>     Monty is now doing refactoring:
> 
>     1. Replacing a lot of LEX_STRING to LEX_CSTRING.
> 
>     2. Fixing functions and methods that have arguments of types:
>     a. pointer "const LEX_STRING *name" and
>     b. reference "const LEX_STRING &name".
>     to the same style. Monty chose passing by pointer (2a).
> 
> 
>     I think that:
> 
>     - #1 is good.
>     - #2 is good, in the sense of changing to the same style.
> 
>     But I think that the choice of "passing by pointer" in #2 is bad.
>     Passing by reference for LEX_STRING/LEX_CSTRING related things would be
>     better.
> 
> 
>     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. 


There are more examples:

  Item_str_func(THD *thd): Item_func(thd) { decimals=NOT_FIXED_DEC; }
  Item_str_func(THD *thd, Item *a): Item_func(thd, a)
{decimals=NOT_FIXED_DEC; }
  Item_str_func(THD *thd, Item *a, Item *b):
    Item_func(thd, a, b) { decimals=NOT_FIXED_DEC; }
  Item_str_func(THD *thd, Item *a, Item *b, Item *c):
    Item_func(thd, a, b, c) { decimals=NOT_FIXED_DEC; }
  Item_str_func(THD *thd, Item *a, Item *b, Item *c, Item *d):
    Item_func(thd, a, b, c, d) { decimals=NOT_FIXED_DEC; }
  Item_str_func(THD *thd, Item *a, Item *b, Item *c, Item *d, Item* e):
    Item_func(thd, a, b, c, d, e) { decimals=NOT_FIXED_DEC; }
  Item_str_func(THD *thd, List<Item> &list):
    Item_func(thd, list) { decimals=NOT_FIXED_DEC; }


This is terrible.

The same thing in Item_int_func, Item_real_func, Item_decimal_func, etc,
etc.

There is only one constructor needed for all of them.


For example, for Item_str_func:


Item_str_func(THD *thd, const Item_args &args):
    Item_func(thd, args) { decimals=NOT_FIXED_DEC; }

And when you need to call new, you do:

  Item *item= new (thd->mem_root) Item_func_somefunc(thd, Item_args(a,b,c));


You cannot do this with the "pass by pointer" approach.


> 
> 
> 
>     I'd propose two things instead:
> 
>     1. Add a class:
> 
> 
> No reason to make a complex solution for a problem that doesn't exist. 


It's a trivial solution.

The problem does exists. The current code absolutely is excessive
and inconvenient to use.


> 
>     2. Instead of "const LEX_CSTRING *str"
>     we use "const Lex_cstring &str" all around C++ code.
> 
> 
> 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. 

I don't propose to use pass-by-value:

func(Some_class arg);


I propose to use pass-by-reference:

func(const Some_class &)



When you do:

 func(some_class_instance);


pass-by-value requires a constructor call, indeed.

but

pass-by-reference does not need a constructor call!



pass-by-reference compiles to exactly the same thing with
pass-by-pointer: it puts the address of the object to the stack.
Nothing else.



> 
> Regards, 
> Monty