Proposal: parsing Qt signals/slots with attributes

Hello,

I would like to add support for recognising Qt’s signal/slot mechanism to clang by using attributes. For that, I have a proposal for a few small changes to the clang parser. These changes use the attributes to mark AST nodes as signal/slot/invokable, and expose those attributes through libclang.

A bit of background: if you are more familiar with Objective-C, then the signal/slot mechanism works nearly exactly the same as Cocoa’s IBAction/IBOutlet: it uses a meta-type system to invoke the methods, or let slots connected to a signal know that the signal was emitted. Qt’s meta-type system also allows for “invokable methods”, which can be called/invoked without being tagged as slot.

Example:

class Test {
public:
Q_INVOKABLE void invokableA();

public slots:
void slotA();
void inlineSlotB() {}

signals:
void signalC();

private:
Q_SIGNAL void signalD(), signalE();
Q_SLOT void slotF(), slotG();
void not_a_slot_or_slot();
Q_SLOT void inlineSlotH() {}
};

(To be used properly with Qt, the class would also have to inherit from QObject, and have the Q_OBJECT macro right after the opening brace.)

Taking a leaf from Cocoa’s book, we can use the pre-processor to inject:

#define signals protected attribute((q_signal))
#define slots attribute((q_slot))
#define Q_SIGNAL attribute((q_signal))
#define Q_SLOT attribute((q_slot))
#define Q_INVOKABLE attribute((q_invokable))

Then the proposed change to clang is to handle it as follows:

  • for method declarations or inline method definitions, accept the q_signal/q_slot/q_invokable attributes, and add the attribute to the AST node
  • for C++ access specifiers, accept an additional (trailing) q_* attribute, and add the attribute to all AST nodes for methods until the next specifier

This is equivalent to how moc (Qt’s Meta-Object Compiler) generates the meta-object for a QObject.

Attached is a proof-of-concept implementation/patch (with the above example as test-case). It also adds the attributes as cursor kinds to libclang, so a client application (like Qt Creator) can use it to find out which of the methods are “tagged” as a signal/slot/invokable

– Erik.

0001-Added-support-for-q_signal-q_slot-and-q_invokable-at.patch (23.1 KB)

Could anybody review this patch? It's very important for Qt community.

Konstantin Tokarev <annulen@yandex.ru>
writes:

I would like to add support for recognising Qt's signal/slot
mechanism to clang by using attributes. For that, I have a proposal
for a few small changes to the clang parser. These changes use the
attributes to mark AST nodes as signal/slot/invokable, and expose
those attributes through libclang.

Could anybody review this patch? It's very important for Qt community.

It seems pretty weird to add special support for a specific GUI library
to clang.

Wouldn't some sort of generic mechanism be more desirable?

[Why is it "very important for Qt community" BTW? Surely they've gotten
along for many years without this support...]

-Miles

Konstantin Tokarev <annulen@yandex.ru>
writes:

šI would like to add support for recognising Qt's signal/slot
šmechanism to clang by using attributes. For that, I have a proposal
šfor a few small changes to the clang parser. These changes use the
šattributes to mark AST nodes as signal/slot/invokable, and expose
šthose attributes through libclang.

šCould anybody review this patch? It's very important for Qt community.

It seems pretty weird to add special support for a specific GUI library
to clang.

Disclaimer: I'm not affiliated with Nokia, and I'm not a developer of Qt (I've
submitted couple of patches though ;). Everything below is my personal
opinion.

Qt is not "a specific GUI library". It's framework for C++ development which
could be used to develop large range of applications including GUI and
non-GUI ones.

Wouldn't some sort of generic mechanism be more desirable?

OK, could you recommend better way to mark methods in "signals" and "slots"
sections with specific attributes without significant performance loss?

[Why is it "very important for Qt community" BTW? šSurely they've gotten
along for many years without this support...]

Clang-based inspection of Qt-based code can bring many new possibilities,
including:

* Reliable code completion in Qt Creator (and maybe other Qt IDEs), taking into
account Qt-specific semantics
* Additional facilities for static analysis of Qt code
* Possibility to write Qt-specific compiler optimizations

Of course it's possible to live without these things, like Apple's implementation of
Objective-C could live without all fancy stuff in clang written exclusively for it.

Erik: Could you bring some light into your future plans on this patch, e.g. are you
planning to maintain this code if it gets into Clang?

Konstantin Tokarev <annulen@yandex.ru>
writes:

Qt is not "a specific GUI library". It's framework for C++ development which
could be used to develop large range of applications including GUI and
non-GUI ones.

Er, .... let's be practical though: these language extensions are _in
practice_ only used by this particular GUI framework/library/
call-it-what-you-will.

Wouldn't some sort of generic mechanism be more desirable?

OK, could you recommend better way to mark methods in "signals" and
"slots" sections with specific attributes without significant
performance loss?

Since your basic goal seems to simply adding new attributes, which are
then stored in the AST, and can be tested for, why not instead a
implement a feature that allows new attributes to be added at runtime?

E.g.,

   #pragma add_attribute oinkarific type

adds the "oinkarific" type attribute.

[Since attributes are a feature shared with gcc, it would be good to
add this in a way that's not inherently clang-specific, so gcc could
later add a similar feature if desired without too much ugliness.]

-Miles

Miles Bader <miles@gnu.org> writes:

Since your basic goal seems to simply adding new attributes, which are
then stored in the AST, and can be tested for, why not instead a
implement a feature that allows new attributes to be added at runtime?

E.g.,

   #pragma add_attribute oinkarific type

adds the "oinkarific" type attribute.

[Since attributes are a feature shared with gcc, it would be good to
add this in a way that's not inherently clang-specific, so gcc could
later add a similar feature if desired without too much ugliness.]

Hmm, or an alternative: just add a set of "tag" attributes, which
don't even need to be declared.

E.g.,:

#define q_variable __attribute__ ((__variable_tag__ "qt_rules"))

   int my_variable q_variable;

["variable_tag" instead of just "tag", so that a macro expanding into
that attribute can be checked by the compiler as being used in the
right context.]

.. with corresponding "type_tag", etc. attributes.

[Hmm, I didn't check; maybe such attributes already exist!]

-Miles

This feature would be very nice, however, it doesn't solve whole problem.

Here's declaration of generic Qt class

class QtClass : public QObject
{
    Q_OBJECT // This is a macro used for code generation and inserting
                       // some common code; slots and signal won't work without it
public:
    QtClass();
    // some other stuff

public slots: // slots is a macro expanding to whatever we want
    slotA();
    slotB();

private slots:
    slotC();
    slotD();

signals: // signals is a macro expanding to whatever we want
    signal1();
    signal2();
    signal3();

// some other stuff
};

It's needed to assign specific attributes to all slots and signals.

Konstantin Tokarev <annulen@yandex.ru>
writes:

This feature would be very nice, however, it doesn't solve whole problem.

Here's declaration of generic Qt class

...

Ok, but I guess my point is: instead of adding qt-specific attributes,
try to figure out some useful generic mechanism, that _can_ be used to
support these qt features without explicitly adding qt support to
clang...

-Miles

I'm not sure what you mean by this. There isn't any fancy stuff in clang that is exclusive to Apple's implementation of Objective-C - clang supports the (GNUstep) open source implementation of Objective-C to the same degree that it supports Apple's.

The closest thing that comes to mind is the handling of IBOutlet and IBAction. These were macros in OpenStep and with clang can expand to attributes, but they have no effect on compilation.

It would help if you could explain what you want to use these attributes for (i.e. why would clang supporting them in the parser help you). The IBAction / IBOutlet support is there to allow the various ad-hoc Objective-C parsers that understood these macros to continue to understand them when switching to libclang. What tools does Qt use that need to understand these annotations?

David

"signals" and "slots" annotations are currently used by Meta-Object Compiler (code generating tool which is part of Qt).

Attribute-based annotations will be used by Qt Creator to provide code completetion, and it can be used in other tools (e.g. code analysis tools) which need to inspect Qt code and use advanced Clang capabilities in the same time.

I haven't looked at the patch in question, but it seems reasonable to add some sort of qt_foobar attribute if it substantially improves QT development.

Random thought experiment: would it be possible to kill off MOC entirely (at least when building with Clang)?

-Chris

In theory, easily so. MOC just writes out C++ code which is then compiled in turn, so a Clang extension could just generate the relevant AST in memory and compile it along with the primary class.

It would be even better to have this as an AST-processing plugin, though.

Sebastian

You've read my thoughts! I have such an experiment in my plans.

And when compiling with other compilers, it would be possible to use Clang-based moc instead of original to generate code. Original MOC has a number of limitations because it actually does build AST [1]. Clang-based moc could overcome them while preserving compatibility with non-Clang compilers.

[1] http://doc.qt.nokia.com/latest/moc.html#limitations

Makes sense, a clang-based rewriter would be a straight-forward way to implement this.

We really need to split them out into plugins though :slight_smile:

-Chris

Well, from a high level: yes. But as always, it is in the details. One of the obviously missing things is an attribute for use in/with the Q_OBJECT macro. For my current use-case (the Qt Creator IDE) I/we do not need that (yet). But a/the more hairy one is Q_PROPERTY:

class Foo: public QObject { Q_OBJECT
    Q_PROPERTY(int width READ width WRITE setWidth NOTIFY widthChanged)
};

For the compiler, this macro expands to empty. But moc actually parses it. I implemented it with in Qt Creator for our current parser, and the syntax is roughly that after the lparen there has to be a (variable) declaration, the parameter-pack for the read/write/notify method is optional (but for the write/notify method it can be a maximum of 1). So this would require to either properly parse it, or to be able to get the macro contents (with the macro recorder, which is currently only used when the parser is instantiated by libclang) and then re-parse it in a plug-in. (Oh, and the read/write/notify methods/slots have to be declared, obviously.)

And no, I did not want to do that (yet?)...

That said, the Q_PROPERTY is probably the most complex case. So after tackling this case, AST generation would not be rocket-science (nor brain surgery).

-- Erik.

I'm not sure what you mean by this. There isn't any fancy stuff in clang that is exclusive to Apple's implementation of Objective-C - clang supports the (GNUstep) open source implementation of Objective-C to the same degree that it supports Apple's.

The closest thing that comes to mind is the handling of IBOutlet and IBAction. These were macros in OpenStep and with clang can expand to attributes, but they have no effect on compilation.

Bingo (see below).

It would help if you could explain what you want to use these attributes for (i.e. why would clang supporting them in the parser help you). The IBAction / IBOutlet support is there to allow the various ad-hoc Objective-C parsers that understood these macros to continue to understand them when switching to libclang. What tools does Qt use that need to understand these annotations?

Like I said in the original email: we prototyped/implemented an integration of clang into Qt Creator as a replacement code model. One of the missing pieces is to have context-sensitive completion for signals/slots while using clang. For example, when doing code-completion with the current code model for:

    connect(theButton, SIGNAL(<here>

or:

    connect(theButton, SIGNAL(clicked()), this, SLOT(<here>

Qt Creator filters the results to only show signals (in the first case) or slots (in the second case). Now to do this, Qt Creator needs to know if completion result (meaning: a declaration) is tagged as either a signal or a slot (or neither).

The patch implements this by doing the same as IBAction/IBOutlet for method declarations/definitions. However, Qt/moc allows for defining whole groups of methods as signals/slots by using the "signals" access specifier (which normally gets expanded to "protected") and "public/protected/private slots" (where slots gets expanded to empty). And this is also the biggest difference with IBAction/IBOutlet. So the patch also handles:

class Checkbox: public QObject {
protected __attribute__((q_signal)): // expanded from "signals"
  void checked();
public __attribute__((q_slots)): // expanded from "public slots"
  void setChecked(bool checked);
};

For the access specifiers, it propagates the attribute to all method declarations following it.

And just like the IBAction/IBOutlet, the attributes are exposed through libclang.

One reason to not add more Q_* macro handling is that Qt Creator does not need it (yet), and because I would like to have it in clang 3.0 (so I can tell users to download that release instead of TOT). So this is a minimal patch to make things work.

Oh, and yes, I work for Nokia as part of the Qt Creator team.

-- Erik.

Hi Erik,

Hello,

I would like to add support for recognising Qt's signal/slot mechanism to clang by using attributes. For that, I have a proposal for a few small changes to the clang parser. These changes use the attributes to mark AST nodes as signal/slot/invokable, and expose those attributes through libclang.

A bit of background: if you are more familiar with Objective-C, then the signal/slot mechanism works nearly exactly the same as Cocoa's IBAction/IBOutlet: it uses a meta-type system to invoke the methods, or let slots connected to a signal know that the signal was emitted. Qt's meta-type system also allows for "invokable methods", which can be called/invoked without being tagged as slot.

Example:

class Test {
public:
    Q_INVOKABLE void invokableA();

public slots:
    void slotA();
    void inlineSlotB() {}

signals:
    void signalC();

private:
    Q_SIGNAL void signalD(), signalE();
    Q_SLOT void slotF(), slotG();
    void not_a_slot_or_slot();
    Q_SLOT void inlineSlotH() {}
};

(To be used properly with Qt, the class would also have to inherit from QObject, and have the Q_OBJECT macro right after the opening brace.)

Taking a leaf from Cocoa's book, we can use the pre-processor to inject:

#define signals protected __attribute__((q_signal))
#define slots __attribute__((q_slot))
#define Q_SIGNAL __attribute__((q_signal))
#define Q_SLOT __attribute__((q_slot))
#define Q_INVOKABLE __attribute__((q_invokable))

Then the proposed change to clang is to handle it as follows:
- for method declarations or inline method definitions, accept the q_signal/q_slot/q_invokable attributes, and add the attribute to the AST node
- for C++ access specifiers, accept an additional (trailing) q_* attribute, and add the attribute to all AST nodes for methods until the next specifier

This is equivalent to how moc (Qt's Meta-Object Compiler) generates the meta-object for a QObject.

Attached is a proof-of-concept implementation/patch (with the above example as test-case). It also adds the attributes as cursor kinds to libclang, so a client application (like Qt Creator) can use it to find out which of the methods are "tagged" as a signal/slot/invokable

I think this is a great direction. Qt is clearly important in the open-source community, and there are a number of technologies (Qt Creator and Qt MOC, at least) that would benefit from better Clang support, and Clang in turn would benefit from the involvement of the Qt community.

A few comments on the patch itself:

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index 4d9aa5b..e6f4119 100644
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -647,3 +647,15 @@ def SharedLocksRequired : InheritableAttr {
   let Args = [VariadicExprArgument<"Args">];
   let LateParsed = 1;
}

A few comments on the patch itself:

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index 4d9aa5b..e6f4119 100644
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -647,3 +647,15 @@ def SharedLocksRequired : InheritableAttr {
  let Args = [VariadicExprArgument<"Args">];
  let LateParsed = 1;
}
+
+def QInvokable : InheritableAttr {
+ let Spellings = ["q_invokable"];
+}
+
+def QSignal : InheritableAttr {
+ let Spellings = ["q_signal"];
+}
+
+def QSlot : InheritableAttr {
+ let Spellings = ["q_slot"];
+}

Why the "q_" prefer rather than "qt_"? Is it because of the Q_-prefixed macros? I ask because "qt_" seems more descriptive.

Yes, Qt uses Q_blah macros. Then again, Clang is not Qt, and I do not have a strong opinion on this. Should I change the attribute names, and if so, should I also change the enumerators in CXCursorKind to read CXCursor_QtSignalAttr, etc?

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index d32c36e..85ad602 100644
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -1047,7 +1047,8 @@ private:
  void DeallocateParsedClasses(ParsingClass *Class);
  void PopParsingClass(Sema::ParsingClassState);

- Decl *ParseCXXInlineMethodDef(AccessSpecifier AS, ParsingDeclarator &D,
+ Decl *ParseCXXInlineMethodDef(AccessSpecifier AS, AttributeList *QtAttrs,
+ ParsingDeclarator &D,
                                const ParsedTemplateInfo &TemplateInfo,
                                const VirtSpecifiers& VS, ExprResult& Init);

I'd rather call this "AccessAttrs" rather than "QtAttrs". Open up a new place for people to add attributes, and they'll find new (non-Qt-related) reasons to add attributes there :slight_smile:

Ok.

@@ -2131,10 +2138,19 @@ void Parser::ParseCXXMemberSpecification(SourceLocation RecordLoc,
        CurAS = AS;
        SourceLocation ASLoc = Tok.getLocation();
        ConsumeToken();
- if (Tok.is(tok::colon))
- Actions.ActOnAccessSpecifier(AS, ASLoc, Tok.getLocation());
- else
+
+ xsAttrs.clear();
+ MaybeParseGNUAttributes(xsAttrs);
+
+ if (Tok.is(tok::colon)) {
+ Decl *xsDecl = Actions.ActOnAccessSpecifier(AS, ASLoc,
+ Tok.getLocation());
+ if (AttributeList *Attrs = xsAttrs.getList())
+ Actions.ProcessDeclAttributeList(Actions.CurScope, xsDecl, Attrs,
+ false, true);
+ } else {
          Diag(Tok, diag::err_expected_colon);
+ }

Could we do some more validation here, so that we reject all attributes *except* those explicitly marked as being okay for access specifiers?

Sure.

Thanks, I will create a new patch including this feedback.

-- Erik.

Honestly, this seems like the perfect use-case for attribute
namespaces (e.g. qt::signal). However, these aren't well-supported
yet. I'd much prefer if we could use these though, since it would
avoid starting a bad habit where it's avoidable.

Sean