Objective-C @defs support

This patch adds parsing and code generation support for @defs directives (which allow you to create a C structure with the same layout as an Objective-C object).

David

defs.diff (7.41 KB)

The patch looks great, some minor issues:

+++ lib/Parse/ParseDecl.cpp (working copy)

Please update the grammar to indicate the changes you're making to the parser.

@@ -736,18 +736,33 @@
+ if(!Tok.is(tok::at)) {

Space after the 'if' please.

...

+ } else { // Handle @defs
+ ConsumeToken();
+ if (!Tok.isObjCAtKeyword(tok::objc_defs))
+ Diag(Tok, diag::err_unexpected_at);

Ok.

+ ConsumeToken();

Not ok. This consumes the token after the @ even though you don't know what it is. If it is <eof>, you're hosed :). You need to think of a recovery strategy here, what should happen if an "@" is seen without defs after it?

+ if (!Tok.is(tok::l_paren)) Diag(Tok, diag::err_expected_lparen);
+ ConsumeParen();

Likewise, and below. Also, please use ExpectAndConsume where possible.

+ llvm::SmallVector<DeclTy*, 16> Fields;
+ Actions.ActOnDefs(CurScope, Tok.getLocation(), Tok.getIdentifierInfo(),
+ Fields);
+ FieldDecls.insert(FieldDecls.end(), Fields.begin(), Fields.end());
+ ConsumeToken();
+ if (!Tok.is(tok::r_paren)) Diag(Tok, diag::err_expected_rparen);
+ ConsumeParen();
+ }

+++ lib/Sema/SemaDecl.cpp (working copy)

+static void CollectIvars(ObjCInterfaceDecl *Class,
+ llvm::SmallVector<Sema::DeclTy*, 16> &ivars) {
+ if(Class->getSuperClass()) {
+ CollectIvars(Class->getSuperClass(), ivars);
+ }
+ for (ObjCInterfaceDecl::ivar_iterator iter=Class->ivar_begin() ;
+ iter!=Class->ivar_end() ; iter++) {
+ ivars.push_back(*iter);
+ }

Can this be: ivars.append(Class->ivar_begin(), Class->ivar_end())?

+}

This patch adds parsing and code generation support for @defs directives (which allow you to create a C structure with the same layout as an Objective-C object).

The patch looks great, some minor issues:

+++ lib/Parse/ParseDecl.cpp (working copy)

Please update the grammar to indicate the changes you're making to the parser.

Done.

@@ -736,18 +736,33 @@
+ if(!Tok.is(tok::at)) {

Space after the 'if' please.

Done.

+ ConsumeToken();

Not ok. This consumes the token after the @ even though you don't know what it is. If it is <eof>, you're hosed :). You need to think of a recovery strategy here, what should happen if an "@" is seen without defs after it?

Fixed

+ if (!Tok.is(tok::l_paren)) Diag(Tok, diag::err_expected_lparen);
+ ConsumeParen();

Likewise, and below. Also, please use ExpectAndConsume where possible.

+ llvm::SmallVector<DeclTy*, 16> Fields;
+ Actions.ActOnDefs(CurScope, Tok.getLocation(), Tok.getIdentifierInfo(),
+ Fields);
+ FieldDecls.insert(FieldDecls.end(), Fields.begin(), Fields.end());
+ ConsumeToken();
+ if (!Tok.is(tok::r_paren)) Diag(Tok, diag::err_expected_rparen);
+ ConsumeParen();
+ }

Done. Lots of errors happen now with nonsense input.

+++ lib/Sema/SemaDecl.cpp (working copy)

+static void CollectIvars(ObjCInterfaceDecl *Class,
+ llvm::SmallVector<Sema::DeclTy*, 16> &ivars) {
+ if(Class->getSuperClass()) {
+ CollectIvars(Class->getSuperClass(), ivars);
+ }
+ for (ObjCInterfaceDecl::ivar_iterator iter=Class->ivar_begin() ;
+ iter!=Class->ivar_end() ; iter++) {
+ ivars.push_back(*iter);
+ }

Can this be: ivars.append(Class->ivar_begin(), Class->ivar_end())?

Yes - it was a loop so I could dump the ivar names as they were found and check they were in the right order.

+}
+
+void Sema::ActOnDefs(Scope *S, SourceLocation DeclStart, IdentifierInfo
+ *ClassName, llvm::SmallVector<DeclTy*, 16> &Decls) {
+ ObjCInterfaceDecl *Class = getObjCInterfaceDecl(ClassName);
+ if (!Class) Diag(DeclStart, diag::err_undef_interface, ClassName->getName());
+ CollectIvars(Class, Decls);
+}
+

comments please!

Added.

+++ lib/CodeGen/CodeGenTypes.cpp (working copy)
@@ -149,7 +149,7 @@

What is this patch doing? It seems unrelated.

Hmm, in fact it might be. Ignore it for now (or commit it separately). It's just a few cleanups of the CodeGenTypes stuff.

defs.m is the file I am using to test this. It outputs two lines in the form %d: %d. If both numbers on each line match then it worked. Compile with ccc defs.m -lobjc. Advice on how to turn this into an automated clang test welcome...

David

defs.diff (6.21 KB)

defs.m (433 Bytes)

+++ lib/CodeGen/CodeGenTypes.cpp (working copy)
@@ -149,7 +149,7 @@

What is this patch doing? It seems unrelated.

Hmm, in fact it might be. Ignore it for now (or commit it separately). It's just a few cleanups of the CodeGenTypes stuff.

Ok, I committed your patch here, thanks!
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20080616/006205.html

defs.m is the file I am using to test this. It outputs two lines in the form %d: %d. If both numbers on each line match then it worked. Compile with ccc defs.m -lobjc. Advice on how to turn this into an automated clang test welcome...

The clang regtest suite isn't the place to test all possible details of the front-end, so I just wanted a test that verifies parser/sema won't regress on this feature. Your example does nicely, because if the @defs don't get added correctly, the field access turns into an error.

Thanks David,

-Chris