CODEWITHSUNDEEP
c++qt
Rob
Rob

Reputation: 78658

Virtual Qt signal?

Whilst reviewing some Qt C++ code I came across this:

class Foo
{
  Q_OBJECT

signals:
  virtual void someSignal(const QString& str, int n)
  {
    Q_UNUSED(str);
    Q_UNUSED(n);
  }
  ...
};

Now, Qt signals cannot have a body so I'm surprised this even compiles (perhaps because the body is effectively empty). I also don't see the point of making a signal virtual as ... it can't have a body so how can it be overridden?

Am I missing something here or is this a valid code smell?

Upvotes: 19

Views: 16551

Answers (4)

Kuba hasn't forgotten Monica
Kuba hasn't forgotten Monica

Reputation: 98425

TL;DR: I don't know what the code was meant to do, but it's wrong (not merely smelling wrong, it's prima facie invalid). Signal implementations are always meant to be generated by moc. The body of the signal should be removed.

  1. For the code to work, it should do all three: compile, get past moc, and link. It is true that your code does compile - the C++ compiler has no reason not to. But it won't pass through moc nor will it link.

  2. Although perhaps moc didn't detect some of it back in 2010, here's how moc acts today:

    a) moc doesn't allow signal definitions in class bodies, with the diagnostic Error: Not a signal declaration. So class X { Q_SIGNAL void s() {} }; triggers it, but class X { Q_SIGNAL void s(); }; void X::s() {} won't.

    b) moc doesn't allow a Q_OBJECT macro in a class not deriving from QObject, with the diagnostic Error: Class contains Q_OBJECT macro but does not inherit from QObject.

  3. Since it doesn't make any sense to talk about signals in classes that don't derive from QObject, let's assume that the code really looked as follows:

    class Foo : public QObject
{
  Q_OBJECT
signals:
  virtual void someSignal(const QString&, int);
};
void Foo::someSignal(const QString& str, int n)
{
  Q_UNUSED(str);
  Q_UNUSED(n);
}

    This will get past moc and will compile, but it won't link. The linker will issue a diagnostic for multiple declaration of Foo::someSignal. There's one definition in this file, and another in the moc-generated source.

Upvotes: 3

gshep
gshep

Reputation: 638

Qt signals are not allowed to be (pure) virtual. See comments to this bug - https://bugreports.qt.io/browse/QTBUG-41004

Upvotes: 3

gregseth
gregseth

Reputation: 13408

Strictly C++ speaking it's normal it compiles, given signal is a macro for protected and Q_UNUSED is a cast to void. But you should get an error when running moc which precisely creates the implementation of the methods declared as signals.

Upvotes: 5

Gareth Stockwell
Gareth Stockwell

Reputation: 3122

That looks smelly to me.

It's valid to declare a signal in a base class and then emit it from a derived class, e.g.

class MyBase : public QObject
{
    Q_OBJECT
// ...
signals:
    void somethingHappened();
};

class MyDerived : public MyBase
{
    Q_OBJECT
// ...
    void doSomething();
};

void MyDerived::doSomething()
{
    // ....
    emit somethingHappened();
}

Maybe that's what the declaration in the question was meant to achieve.

Upvotes: 23

Related Questions