[cfe-dev] Proposal: parsing Qt signals/slots with attributes

[Douglas Gregor]

Hi Erik,

On Sep 20, 2011, at 1:10 AM, erikjv wrote:

> 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;
 }
+
+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.

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 :)

@@ -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?

Otherwise, looks good! 

	- Doug