Pelagicore / gdbus-codegen-glibmm

Code generator for C++ D-Bus stubs and proxies using Giomm/Glibmm
GNU Lesser General Public License v2.1
23 stars 25 forks source link

templates: Use references when passing around Glib::RefPtr #73

Closed martin-ejdestig closed 5 years ago

martin-ejdestig commented 5 years ago

To avoid unnecessary reference counting. MethodInvocation has a Glib::RefPtr member, so pass that as a reference as well.

Fixes #58.

mardy commented 5 years ago

Can you explain a bit better why you need the temporary variable again? By looking at the code, the two variants seem to be completely equivalent; am I missing something?

martin-ejdestig commented 5 years ago

Can you explain a bit better why you need the temporary variable again? By looking at the code, the two variants seem to be completely equivalent; am I missing something?

You are not allowed to pass an rvalue for a non-const reference argument. I forget the details as to why and could not find any good explanation when searching quickly. But e.g.:

#include <iostream>

struct Bar
{
    int i = 0;
};

static void foo(Bar &bar)
{
    std::cout << bar.i << '\n';
}

int main()
{
    foo(Bar());
    return 0;
}

Will give you a compilation error:

delme.cpp:15:6: error: cannot bind non-const lvalue reference of type ‘Bar&’ to an rvalue of type ‘Bar’
  foo(Bar())

Changing void foo(Bar &bar) to void foo(const Bar &bar) or creating a variable in main() that is passed to foo() will work.

martin-ejdestig commented 5 years ago

Forgot to update example in README.md. Fixed now.

martin-ejdestig commented 5 years ago

I forget the details as to why and could not find any good explanation when searching quickly.

Did a quick search again and found a blurb in Stroustrup's C++11 FAQ ( http://www.stroustrup.com/C++11FAQ.html#rval ):

"In C++, non-const references can bind to lvalues and const references can bind to lvalues or rvalues, but there is nothing that can bind to a non-const rvalue. That's to protect people from changing the values of temporaries that are destroyed before their new value can be used."

So, the reason is that it does not make sense. :)

In our case here, it does not really make sense either if you know that MethodInvocation only stores a RefPtr. But from the user's perspective, it looks a bit weird to get a const MethodInvocation &. But, I do not know. I went back and forth on it.