Re: [Development] invokeMethod() with function pointers

2017-01-24 Thread Benjamin TERRIER
Hi,

I'd like to get this in before 5.9 FF.

The current state is:
 - It works for member functions, function pointers and functors
 - It soft breaks existing code that were passing null literals.
 - The new functions do not accepts any arguments, users have to use lambda
 - The new functions do accept an optional return argument in the form
"ReturnType *" (without using Q_ARG),
the type is checked during compilation for function pointers (member or
not) using QtPrivate::FunctionPointer.
I am not sure for functors though as QtPrivate::FunctionPointer does not
work in this case, but QFunctorSlotObject
ensure that the actual return type of the functor can be assigned to a
ReturnType.

What must be done:
 - Add documentation
 - Complete auto tests
 - Optionally, remove the "ReturnType *" argument and for users to use
lambda with capture.
This remove the need to check for return in case of queued connection, but
add the responsibility for the user to ensure the lifespace
of variable captured by reference to sill be valid when the lambda is
executed.
- Optionally, move the function pointer/lambda to the last argument
position. This can make nicer code when using lambda,
at the cost of having to provide more overloads instead of using optional
arguments.

I would appreciate if any of you could check the gerrit change (
https://codereview.qt-project.org/#/c/182339/) and provide feedbacks,
especially if there is anything that would require a major change of the
current state.

Also, I'd like to get more opinions about the 2 optional changes.

Thanks.

Regards,

Benjamin
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-21 Thread Grégoire Barbier

Le 21/01/2017 à 00:20, Thiago Macieira a écrit :

It's a shift of responsibility. If we had the BlockingAutoConnection (or
SynchronousConnection, which is what I wanted to call it during Qt 4.5 when I
first proposed it), the deadlock would be Qt's fault. Since it's you doing
that, the deadlock becomes your fault and you're the one who needs to fix it.


Sorry I didn't realized until reading this that the deadlock was in the 
if itself, it's stupid. So of course I agree to not shift the 
responsibilities and will keep writing my own ifs (where they are safe).


Anyway the other ideas in the thread are still interesting IMO.
Thanks.

--
Grégoire Barbier :: g à g76r.eu :: +33 6 21 35 73 49
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-20 Thread Thiago Macieira
On sexta-feira, 20 de janeiro de 2017 17:34:50 PST Grégoire Barbier wrote:
> > See the discussion in https://codereview.qt-project.org/83404/ for why
> > this is not a good idea.
> > In summary, BlockingQueuedConnection is dangerous as it can lead to
> > deadlock if the other thread is waiting on your thread. In order to use
> > it properly, you really need to know, while programming in which thread
> > you are and which thread you try to communicate to, and be sure that
> > there is never in your program cases where the other thread may be
> > waiting on a QWaitCondition for your other thread. So this pattern with
> > QThread::currentThread() == this-> 
> >>thread() before a BlockingQueuedConnection is really dangerous.

> I respect the fact that you rejected Qt::BlockingAutoConnection in 2014
> because of its potential danger, but I'm not sure that it's better to
> let people use the "QThread::currentThread() == this" pattern without
> being warned rather than implementing Qt::BlockingAutoConnection, with a
> detailed warning in the doc.

It's a shift of responsibility. If we had the BlockingAutoConnection (or 
SynchronousConnection, which is what I wanted to call it during Qt 4.5 when I 
first proposed it), the deadlock would be Qt's fault. Since it's you doing 
that, the deadlock becomes your fault and you're the one who needs to fix it.

> Moreover, I don't understand whi BlockingAutoConnection would be more
> dangerous (if it were to be added) than BlockingQueuedConnection is
> currently already dangerous.

Think about this: QueuedConnection and BlockingQueuedConnection chase the 
target object across moveToThreads. That is, if thread A is doing 
invokeMethod() while thread B does obj->moveToThread(C), then the invocation 
chases obj to thread C. That's a good thing.

But what happens if it got moved to thread A? Since thread A is blocked on a 
semaphore wait, it can't handle the incoming metacall event. That's a 
deadlock.

What's more, we can't solve it: even if we could detect the deadlock formation 
before it happens (and I'm not sure we can), we'd need to pick the metacall 
event out of the event queue, which means it could jump the queue and things 
would execute in the wrong order. So, no, it's impossible to implement 
BlockingAutoConnection so long as the object can be moved back to the calling 
thread.

> With your own words from 2014:
> 
> """
> I would not even try to solve this race with moveToThead, as a mention,
> i think we just need to document that one must not call moveToThread
> when Blocking connecitona re involved.
> """
> 
> Maybe it would be interesting to set this comment in enum
> Qt::ConnectionType doc, just right to Qt::BlockingQueuedConnection (or a
> would-be Qt::BlockingAutoConnection) and not only in the "Signals and
> Slots Across Threads" paragraph, to make everything safer with noobs
> (like me) that do not read the whole docs but only spam the F1 key from
> with Qt Creator.

Adding the warning to BlockingQueuedConnection is a good idea.

> > On the other hand, it would be useful to get the return value of a
> > QueuedConnection in a QFuture.

That's no different than a QueuedConnection with a posted event back with the 
result. In fact, we have a very good implementation for "post event back with 
result": we call it a "signal-slot connection".

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-20 Thread Gunnar Roth

> 
> I respect the fact that you rejected Qt::BlockingAutoConnection in 2014 
> because of its potential danger, but I'm not sure that it's better to let 
> people use the "QThread::currentThread() == this" pattern without being 
> warned rather than implementing Qt::BlockingAutoConnection, with a detailed 
> warning in the doc.
> 
> Moreover, I don't understand whi BlockingAutoConnection would be more 
> dangerous (if it were to be added) than BlockingQueuedConnection is currently 
> already dangerous.

I also don’t understand why adding  "QThread::currentThread() == this“ should 
make the method more dangerous, actually it makes it a bit more safe for the 
same thread case, the potential deadlock problem remains with or without.

Regards,
Gunnar Roth 
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-20 Thread Grégoire Barbier

Le 20/01/2017 à 11:14, Olivier Goffart a écrit :

On Dienstag, 17. Januar 2017 11:21:56 CET Grégoire Barbier wrote:

Le 16/01/2017 à 10:34, Olivier Goffart a écrit :
> What's the use case for this function? For direct call you better of
> calling the function directly,  and the equivalent of QueuedConnection
> can be achieved with QTimer::singleShot.

Hi.

AFAIK there is no other way to call a method across threads *and wait
for it result* than QMetaObject::invokeMethod() with
Qt::BlockingQueuedConnection and Q_RETURN_ARG (apart, of course, making
the called method thread-safe).


That's a valid use case. Thank you for bringing that up.

[...]


Also (I still dream), if there was a way to make invokeMethod()
automagically choose between a direct call and
Qt::BlockingQueuedConnection, it would be possible to get rid of this idiom:
if (QThread::currentThread() == this->thread())
foo = func();
 else
QMetaObject::invokeMethod(this, "func",
  Qt::BlockingQueuedConnection,
  Q_RETURN_ARG(foo));

Kind of Qt::DirectOrBlockingQueuedConnection.


See the discussion in https://codereview.qt-project.org/83404/ for why this is
not a good idea.
In summary, BlockingQueuedConnection is dangerous as it can lead to deadlock
if the other thread is waiting on your thread. In order to use it properly,
you really need to know, while programming in which thread you are and which
thread you try to communicate to, and be sure that there is never in your
program cases where the other thread may be waiting on a QWaitCondition for
your other thread. So this pattern with QThread::currentThread() == this-

thread() before a BlockingQueuedConnection is really dangerous.


Thank you it's very interesting.

But, I use this pattern mainly to protect data-access methods (such as a 
getter on an implicitly shared object) than can be either called from 
the owner thread or another one.
I'll keep using it, without fear, unless the called object's owner 
thread was likelly to change.


I respect the fact that you rejected Qt::BlockingAutoConnection in 2014 
because of its potential danger, but I'm not sure that it's better to 
let people use the "QThread::currentThread() == this" pattern without 
being warned rather than implementing Qt::BlockingAutoConnection, with a 
detailed warning in the doc.


Moreover, I don't understand whi BlockingAutoConnection would be more 
dangerous (if it were to be added) than BlockingQueuedConnection is 
currently already dangerous.


With your own words from 2014:

"""
I would not even try to solve this race with moveToThead, as a mention, 
i think we just need to document that one must not call moveToThread 
when Blocking connecitona re involved.

"""

Maybe it would be interesting to set this comment in enum 
Qt::ConnectionType doc, just right to Qt::BlockingQueuedConnection (or a 
would-be Qt::BlockingAutoConnection) and not only in the "Signals and 
Slots Across Threads" paragraph, to make everything safer with noobs 
(like me) that do not read the whole docs but only spam the F1 key from 
with Qt Creator.



On the other hand, it would be useful to get the return value of a
QueuedConnection in a QFuture.


Well.
In french there is a very funny expression which means more or less "to 
use a hammer to kill a fly". ;-)
Let's take an example, I clearly don't think I would like to use a 
QFuture, make it thread-safe with a QMutex and the like in the following 
code (even though you've just make me learn that 
Qt::BlockingQueuedConnection may be dangerous, in the following code I 
know it's not, because A object never changes thread):


class B;
class A : public QObject {
  Q_OBJECT
  QLinkedList _queue;
public:
  /** This method is thread-safe. */
  B pop() const {
B value;
if (QThread::currentThread() == this->thread())
  value = doPop();
else
  QMetaObject::invokeMethod(this, "doFoo",
 Qt::BlockingQueuedConnection,
 Q_RETURN_ARG(B, value));
return value;
  }
private:
  /** This method is not thread-safe. */
  Q_INVOKABLE B doPop() {
return _queue.takeFirst();
  }
};

In the other hand, I would by far prefer to write this code, both the 
lambda and Qt::AutoBlockingConnection make the code shorter and easier 
to understand (and I don't expect that using QFuture would lead to the 
same result):


class B;
class A : public QObject {
  Q_OBJECT
  QLinkedList _queue;
public:
  /** This method is thread-safe. */
  B pop() const {
 B value;
 QMetaObject::invokeMethod(this, []() {
 value = _queue.takeFirst();
 }, Qt::AutoBlockingConnection);
 }
  }
};

Of course in this simple case, it's obvious that make the method 
thread-safe (e.g. with a QMutex/QMutexLocker) is easier and lighter than 
a queued call. But it's not always so obvious.


--
Grégoire Barbier :: g à g76r.eu :: +33 6 21 35 73 49
___
Development mailing list

Re: [Development] invokeMethod() with function pointers

2017-01-20 Thread Olivier Goffart
On Dienstag, 17. Januar 2017 11:21:56 CET Grégoire Barbier wrote:
> Le 16/01/2017 à 10:34, Olivier Goffart a écrit :
> > What's the use case for this function? For direct call you better of
> > calling the function directly,  and the equivalent of QueuedConnection
> > can be achieved with QTimer::singleShot.
> 
> Hi.
> 
> AFAIK there is no other way to call a method across threads *and wait
> for it result* than QMetaObject::invokeMethod() with
> Qt::BlockingQueuedConnection and Q_RETURN_ARG (apart, of course, making
> the called method thread-safe).

That's a valid use case. Thank you for bringing that up.

[...]

> Also (I still dream), if there was a way to make invokeMethod()
> automagically choose between a direct call and
> Qt::BlockingQueuedConnection, it would be possible to get rid of this idiom:
> if (QThread::currentThread() == this->thread())
> foo = func();
>  else
> QMetaObject::invokeMethod(this, "func",
>   Qt::BlockingQueuedConnection,
>   Q_RETURN_ARG(foo));
> 
> Kind of Qt::DirectOrBlockingQueuedConnection.

See the discussion in https://codereview.qt-project.org/83404/ for why this is 
not a good idea.
In summary, BlockingQueuedConnection is dangerous as it can lead to deadlock 
if the other thread is waiting on your thread. In order to use it properly, 
you really need to know, while programming in which thread you are and which 
thread you try to communicate to, and be sure that there is never in your 
program cases where the other thread may be waiting on a QWaitCondition for 
your other thread. So this pattern with QThread::currentThread() == this-
>thread() before a BlockingQueuedConnection is really dangerous.


On the other hand, it would be useful to get the return value of a 
QueuedConnection in a QFuture.


-- 
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org




___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-20 Thread Olivier Goffart
On Freitag, 20. Januar 2017 09:59:55 CET Benjamin TERRIER wrote:
> 2017-01-20 3:01 GMT+01:00 Thiago Macieira :
> > we also catch the even more dubious code:
> > char func[] = "deleteLater";
> > QMetaObject::invokeMethod(, func);

I think we should still support that.

> > 
> > The only case that would break would be for a constant null pointer
> > literal, which in my opinion is acceptable, since it should never happen
> > in real code.

I'd say breaking with 0 or nullptr is an acceptable source compatibility break 
since this makes no sens.


> As an alternative to adding "char *" overload, it seems that replacing
> "std::is_same"
> by "std::is_same" in the "QtPrivate::QEnableIf" part of
> the template solves the issue
> for char * and const char *.

Maybe we need a smarter condition in the enable_if like is_callable or
!std::is_convertible

> Also I have checked QTimer and QTimer::singleShot(0, , (char *)0);
> doesn't compile because
> the "wrong" overload is chosen. Removing the const in the std::is_same
> should also fix this.

this overload of QTimer::singleShot is meant to be used with the SLOT macro, 
so that's only a "const char*".  Nobody ever complained about that since Qt 
5.4 when it was added, so i think we could leave it like that.

But if you want to "fix" it there, there is also QMenu::addAction and 
QToolbar::addAction

-- 
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-20 Thread Benjamin TERRIER
2017-01-20 3:01 GMT+01:00 Thiago Macieira :
> Because it's a template, so the template when Func = char* matches better than
> the overload with const char*. I assume that using nullptr without casting
> also breaks, correct?

Correct.

> From what you explained, this will not affect the case when the pointer is a
> const char *, so neither
>
> const char *func = "deleteLater";
> QMetaObject::invokeMethod(, func);
>
> nor
>
> const char *func = nullptr;
> QMetaObject::invokeMethod(, func);
>
> will stop compiling. So if you add an overload taking a non-const char* (and
> its unit test), we also catch the even more dubious code:
>
> char func[] = "deleteLater";
> QMetaObject::invokeMethod(, func);
>
> The only case that would break would be for a constant null pointer literal,
> which in my opinion is acceptable, since it should never happen in real code.

As an alternative to adding "char *" overload, it seems that replacing
"std::is_same"
by "std::is_same" in the "QtPrivate::QEnableIf" part of
the template solves the issue
for char * and const char *.


Also I have checked QTimer and QTimer::singleShot(0, , (char *)0);
doesn't compile because
the "wrong" overload is chosen. Removing the const in the std::is_same
should also fix this.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-19 Thread Thiago Macieira
On quinta-feira, 19 de janeiro de 2017 16:18:30 PST Grégoire Barbier wrote:
> > The return value is interesting still.
> 
> With lambdas the return value itself can be replaced with a captured
> reference, isn't it ?
> Anyway it's still convenient to have it when calling plain old methods
> rather than lambdas.

Hmm... right. What I had proposed:

   QMetaObject::invokeMethod(object, [=]() { 
doSomething(); return something; },
Qt::BlockingQueuedConnection,
Q_RETURN_ARG(foo));

Could be rewritten as:

   QMetaObject::invokeMethod(object, [=, ]() { 
doSomething(); foo = something; },
Qt::BlockingQueuedConnection);

And that allows us to drop the Q_RETURN_ARG ugliness. That's much better.

PS: should we invert the argument order and have the connection type appear 
before the functor? I find it ugly to have a lambda in the middle of a 
parameter list.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-19 Thread Thiago Macieira
On quinta-feira, 19 de janeiro de 2017 12:24:34 PST Benjamin TERRIER wrote:
> template 
> static typename
> QtPrivate::QEnableIf::IsPointerToMemberFun
> ction && QtPrivate::FunctionPointer::ArgumentCount == -1
>   && !std::is_same::value, bool>::Type
>   invokeMethod(QObject *object, Func function)
> 
> to QMetaObject breaks existing code and the auto tests in particular.
> 
> In tst_qmetaobject.cpp there is this test:
> QVERIFY(!QMetaObject::invokeMethod(, 0));
> because the new overload of invokeMethod() gets called this lead to
> compilation error instead
> of just returning false at run-time.
> 
> To solve the issue one must add an explicit cast like so:
> QVERIFY(!QMetaObject::invokeMethod(, (const char*)0));
> Note that casting to "char *" does not solve the issue.

Because it's a template, so the template when Func = char* matches better than 
the overload with const char*. I assume that using nullptr without casting 
also breaks, correct?

> Is this "source break"  acceptable (it might be looking at "Source
> break policy for function overloads" discussion and quip 6) and the
> autotests should be fixed?

>From what you explained, this will not affect the case when the pointer is a 
const char *, so neither

const char *func = "deleteLater";
QMetaObject::invokeMethod(, func);

nor

const char *func = nullptr;
QMetaObject::invokeMethod(, func);

will stop compiling. So if you add an overload taking a non-const char* (and 
its unit test), we also catch the even more dubious code:

char func[] = "deleteLater";
QMetaObject::invokeMethod(, func);

The only case that would break would be for a constant null pointer literal, 
which in my opinion is acceptable, since it should never happen in real code.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-19 Thread Grégoire Barbier

Le 17/01/2017 à 18:11, Thiago Macieira a écrit :

Em terça-feira, 17 de janeiro de 2017, às 11:21:56 PST, Grégoire Barbier
escreveu:

And maybe lambdas too, if there was a way to choose the thread/eventloop
in which we want the lambda to be executed (but christmas was a few
weeks ago, I should not dream ;-)).


If we do this, it should be possible to write:

QMetaObject::invokeMethod(object, [=]() {
doSomething(); return something; },
Qt::BlockingQueuedConnection,
Q_RETURN_ARG(foo));


It would be great. :-)


Since we have lambdas and std::bind, I don't see the point of supporting
passing arguments.


Agree.


The return value is interesting still.


With lambdas the return value itself can be replaced with a captured 
reference, isn't it ?
Anyway it's still convenient to have it when calling plain old methods 
rather than lambdas.



--
Grégoire Barbier :: g à g76r.eu :: +33 6 21 35 73 49
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-19 Thread Benjamin TERRIER
Hi,

I've got an issue.

Adding

template 
static typename
QtPrivate::QEnableIf::IsPointerToMemberFunction
  && QtPrivate::FunctionPointer::ArgumentCount == -1
  && !std::is_same::value, bool>::Type
  invokeMethod(QObject *object, Func function)

to QMetaObject breaks existing code and the auto tests in particular.

In tst_qmetaobject.cpp there is this test:
QVERIFY(!QMetaObject::invokeMethod(, 0));
because the new overload of invokeMethod() gets called this lead to
compilation error instead
of just returning false at run-time.

To solve the issue one must add an explicit cast like so:
QVERIFY(!QMetaObject::invokeMethod(, (const char*)0));
Note that casting to "char *" does not solve the issue.

Is this "source break"  acceptable (it might be looking at "Source
break policy for function overloads" discussion and quip 6) and the
autotests should be fixed?
Or should another solution be found (Thiago proposed to use a
different name in QTBUG-37253)?

Thanks

Regards,

Benjamin
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-17 Thread Thiago Macieira
Em terça-feira, 17 de janeiro de 2017, às 11:21:56 PST, Grégoire Barbier 
escreveu:
> And maybe lambdas too, if there was a way to choose the thread/eventloop
> in which we want the lambda to be executed (but christmas was a few
> weeks ago, I should not dream ;-)).

If we do this, it should be possible to write:

QMetaObject::invokeMethod(object, [=]() { 
doSomething(); return something; },
Qt::BlockingQueuedConnection,
Q_RETURN_ARG(foo));

Since we have lambdas and std::bind, I don't see the point of supporting 
passing arguments. The return value is interesting still.
-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-17 Thread Edward Welbourne
Grégoire Barbier:
> Kind of Qt::DirectOrBlockingQueuedConnection.

Blocking_DirectOrQueued_Connection, surely.

Eddy.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-17 Thread Grégoire Barbier

Le 16/01/2017 à 10:34, Olivier Goffart a écrit :

What's the use case for this function? For direct call you better of calling
the function directly,  and the equivalent of QueuedConnection can be achieved
with QTimer::singleShot.


Hi.

AFAIK there is no other way to call a method across threads *and wait 
for it result* than QMetaObject::invokeMethod() with 
Qt::BlockingQueuedConnection and Q_RETURN_ARG (apart, of course, making 
the called method thread-safe).


I would personally be happy if (like Benjamin proposes) there were some 
compile time check, IDE symbols following/renaming and no longer need to 
declare such methods as Q_INVOKABLE (or slot).
Therefore IMO methods pointers would be great in 
QMetaObject::invokeMethod() as they are in QObject::connect(). :-)


And maybe lambdas too, if there was a way to choose the thread/eventloop 
in which we want the lambda to be executed (but christmas was a few 
weeks ago, I should not dream ;-)).


Also (I still dream), if there was a way to make invokeMethod() 
automagically choose between a direct call and 
Qt::BlockingQueuedConnection, it would be possible to get rid of this idiom:

if (QThread::currentThread() == this->thread())
   foo = func();
else
   QMetaObject::invokeMethod(this, "func",
 Qt::BlockingQueuedConnection,
 Q_RETURN_ARG(foo));

Kind of Qt::DirectOrBlockingQueuedConnection.



--
Grégoire Barbier :: g à g76r.eu :: +33 6 21 35 73 49
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-16 Thread Thiago Macieira
On segunda-feira, 16 de janeiro de 2017 19:23:07 PST Benjamin TERRIER wrote:
> The QTimer solution could work, but you cannot add parameters without using
> std::bind.

Use a lambda.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-16 Thread Benjamin TERRIER
2017-01-16 10:34 GMT+01:00 Olivier Goffart :

> On Samstag, 14. Januar 2017 17:28:01 CET Benjamin TERRIER wrote:
> > Hi everyone,
> >
> > I'm trying to contribute by making QMetaObject::invokeMethod() take
> function
> > pointers instead of function names.
> >
> > I've come up with something that works by looking at the code of
> > QMetaObject::invokeMethod, QObject::connect and QMetaObject::activate.
>
> Thanks for your contribution.
>
> What's the use case for this function? For direct call you better of
> calling
> the function directly,  and the equivalent of QueuedConnection can be
> achieved
> with QTimer::singleShot.
> Nevertheless, i guess it's worth adding for the sake of consistency.
>

Talking for me here, I use QMetaObject::invokeMethod() to call slots of
QObject accross threads.
I could make use of the new form to call any function (not only slots) and
to have some checks during compilation.

The QTimer solution could work, but you cannot add parameters without using
std::bind.

However, I don't really like QGenericArgument which forces to use Q_ARG.
> This
> was just a workaround back in the Qt 4 days, around the lack of template
> member function and variadic template. Since we know the arguments of the
> slot
> at compile time, it would be much nicer to pass the arguments directly.
>

Agreed. I've got something working with variadic templates, I'll update the
change later.

But for consistency, it should stay in QMetaObject, I'd say.  We might need
> to
> move a few more things to qobjectdefs_impl.h to make it happen.


Can this be done in the same commit? Or do I need to make 2 changes?

Benjamin
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-16 Thread Olivier Goffart
On Montag, 16. Januar 2017 12:57:01 CET Konstantin Tokarev wrote:
> 16.01.2017, 12:34, "Olivier Goffart" :
> > On Samstag, 14. Januar 2017 17:28:01 CET Benjamin TERRIER wrote:
> >>  Hi everyone,
> >> 
> >>  I'm trying to contribute by making QMetaObject::invokeMethod() take
> >> function pointers instead of function names.
> >> 
> >>  I've come up with something that works by looking at the code of
> >>  QMetaObject::invokeMethod, QObject::connect and QMetaObject::activate.
> > 
> > Thanks for your contribution.
> > 
> > What's the use case for this function? For direct call you better of
> > calling the function directly, and the equivalent of QueuedConnection can
> > be achieved with QTimer::singleShot.
> 
> QTimer::singleShot creates temporary QSingleShotTimer object, which does not
> seem to be efficient solution if same method needs to be called a lot of
> times in QueuedConnection way.

Yes, that's true: The string based method is optimized for 0ms timeout, but 
not the function pointer one. 
The function pointer version could be optimized for 0 timer the same way that 
the string version of QTimer::singleShot does.
(Well, not exactly the same way currently as we cannot use 
QMetaObject::invokeMethod yet.)

-- 
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-16 Thread Konstantin Tokarev


16.01.2017, 12:34, "Olivier Goffart" :
> On Samstag, 14. Januar 2017 17:28:01 CET Benjamin TERRIER wrote:
>>  Hi everyone,
>>
>>  I'm trying to contribute by making QMetaObject::invokeMethod() take function
>>  pointers instead of function names.
>>
>>  I've come up with something that works by looking at the code of
>>  QMetaObject::invokeMethod, QObject::connect and QMetaObject::activate.
>
> Thanks for your contribution.
>
> What's the use case for this function? For direct call you better of calling
> the function directly, and the equivalent of QueuedConnection can be achieved
> with QTimer::singleShot.

QTimer::singleShot creates temporary QSingleShotTimer object, which does not 
seem to be efficient solution if same method needs to be called a lot of times 
in QueuedConnection way. 

Also, from quick glance it seems like there is no fast path for zero timer 
interval in startTimer(), so internal timer is registered in event loop for 
each call.

> Nevertheless, i guess it's worth adding for the sake of consistency.
>
> However, I don't really like QGenericArgument which forces to use Q_ARG. This
> was just a workaround back in the Qt 4 days, around the lack of template
> member function and variadic template. Since we know the arguments of the slot
> at compile time, it would be much nicer to pass the arguments directly.
>
>>  However it does not check for parameters and it is implemented as a function
>>  of QObject (because I needed QSlotObjects classes which are not available
>>  in qobjectdef.h).
>
> But for consistency, it should stay in QMetaObject, I'd say. We might need to
> move a few more things to qobjectdefs_impl.h to make it happen.
>
>>  It can handle QObject member functions and functors in
>>  which case "this" is used to select the event loop (like connect()
>>  functions).
>>
>>  I've uploaded my change as a draft on gerrit:
>>  https://codereview.qt-project.org/#/c/182339/
>>
>>  Here is the link to the relevant bug report:
>>  https://bugreports.qt.io/browse/QTBUG-37253
>>
>>  I welcome any comment and feedback.
>>
>>  BR,
>>
>>  Benjamin Terrier
>
> --
> Olivier
>
> Woboq - Qt services and support - https://woboq.com - https://code.woboq.org
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development

-- 
Regards,
Konstantin
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-16 Thread Olivier Goffart
On Samstag, 14. Januar 2017 17:28:01 CET Benjamin TERRIER wrote:
> Hi everyone,
> 
> I'm trying to contribute by making QMetaObject::invokeMethod() take function
> pointers instead of function names.
> 
> I've come up with something that works by looking at the code of
> QMetaObject::invokeMethod, QObject::connect and QMetaObject::activate.

Thanks for your contribution.

What's the use case for this function? For direct call you better of calling 
the function directly,  and the equivalent of QueuedConnection can be achieved 
with QTimer::singleShot.
Nevertheless, i guess it's worth adding for the sake of consistency.

However, I don't really like QGenericArgument which forces to use Q_ARG. This 
was just a workaround back in the Qt 4 days, around the lack of template 
member function and variadic template. Since we know the arguments of the slot 
at compile time, it would be much nicer to pass the arguments directly. 

> However it does not check for parameters and it is implemented as a function
> of QObject (because I needed QSlotObjects classes which are not available
> in qobjectdef.h). 

But for consistency, it should stay in QMetaObject, I'd say.  We might need to 
move a few more things to qobjectdefs_impl.h to make it happen.

> It can handle QObject member functions and functors in
> which case  "this" is used to select the event loop (like connect()
> functions).
> 
> I've uploaded my change as a draft on gerrit:
> https://codereview.qt-project.org/#/c/182339/
> 
> Here is the link to the relevant bug report:
> https://bugreports.qt.io/browse/QTBUG-37253
> 
> I welcome any comment and feedback.
> 
> BR,
> 
> Benjamin Terrier


-- 
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-14 Thread Konstantin Tokarev


14.01.2017, 20:42, "Thiago Macieira" :
> On sábado, 14 de janeiro de 2017 17:28:01 PST Benjamin TERRIER wrote:
>>  I've uploaded my change as a draft on gerrit:
>>  https://codereview.qt-project.org/#/c/182339/
>
> No one can see it while it's a draft. You have to publish the change.

Correction: no one except people added as reviewers.

>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development

-- 
Regards,
Konstantin
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-14 Thread Benjamin TERRIER
2017-01-14 18:42 GMT+01:00 Thiago Macieira :

>
> No one can see it while it's a draft. You have to publish the change.
>
>
 Ok, it's done.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] invokeMethod() with function pointers

2017-01-14 Thread Thiago Macieira
On sábado, 14 de janeiro de 2017 17:28:01 PST Benjamin TERRIER wrote:
> I've uploaded my change as a draft on gerrit:
> https://codereview.qt-project.org/#/c/182339/

No one can see it while it's a draft. You have to publish the change.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development