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

[Erik Verbruggen]

On 07 Oct, 2011,at 10:32 PM, Douglas Gregor <dgregor at apple.com> wrote:


On Oct 6, 2011, at 2:12 AM, Erik Verbruggen wrote:

> On 30 Sep, 2011,at 07:34 AM, Douglas Gregor <dgregor at apple.com> wrote:
> 
>> Let's use the existing "annotate" attribute, which is a bit like the "user" attribute mentioned in (2) above, but already implemented in Clang. That way, Qt can have definitions such as:
>> 
>> #define Q_OBJECT __attribute__((annotate("qt_object")))
>> 
>> Then, you patch need only:
>> 
>> (a) add support for parsing attributes on access specifiers, and
>> (b) update libclang to expose the "annotate" attribute 
>> 
>> That should be sufficient, requires much less effort (and risk) than most of the other solutions, and won't require us to bring anything related to Qt into Clang itself.
> 
> Attached is a new patch, which adds parsing of attributes after a C++ access specifier. These attributes get propagated in the same manner as the access specifier.

I see that this handles, e.g.,

public slots:

(with the attribute definition of "slots", of course)

but it doesn't seem to handle, e.g.,

signals:

I assume you want to address that as well?
 
qobjectdefs.h uses:
#   define slots
#   define signals protected

so with attributes one can use:
#   define slots __attribute__((annotate("qt_slot")))
#   define signals protected __attribute__((annotate("qt_signal")))


> Then in c-index-test I changed the spelling of the annotate attribute to return the quoted string.

That makes sense, thanks.

> Question: should I add a check to disallow attributes other than the annotate attribute after an access specifier?

Yes. I'd like us to enable such attributes on a case-by-case basis, rather than deal with the fallout of them all implicitly propagating.
 
Ok, added it.


Minor nit:

@@ -2139,10 +2146,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
+
+ AccessAttrs.clear();
+ MaybeParseGNUAttributes(AccessAttrs);
+
+ if (Tok.is(tok::colon)) {
+ Decl *xsDecl = Actions.ActOnAccessSpecifier(AS, ASLoc,
+ Tok.getLocation());
+ if (AttributeList *Attrs = AccessAttrs.getList())
+ Actions.ProcessDeclAttributeList(Actions.CurScope, xsDecl, Attrs,
+ false, true);
+ } else {
Diag(Tok, diag::err_expected_colon);
+ }

http://llvm.org/docs/CodingStandards.html#ll_naming

suggests that "nsDecl" be named starting with a capital letter.
 
Changed it to AccessDecl.


We also seem to be missing tests.

Should be included now. Attached patch is against r141497.

-- Erik.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20111009/129b2c76/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libclang-Allow-for-annotate-attributes-after-access-.patch
Type: application/octet-stream
Size: 18256 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20111009/129b2c76/attachment.obj>