[cfe-dev] Objective-C Message Send Generation

[Devang Patel]

Hi David,

On Feb 28, 2008, at 7:29 AM, David Chisnall wrote:

> Hi Everyone,
>
> I've started working on Objective-C code generation.  The attached  
> diff contains my initial work.

very good!

>  I've added a few hooks into the code generation code for handling  
> the Objective-C AST nodes.
>
> I've created a CodeGenObjCRuntime abstract class which encapsulates  
> the runtime-specific components and a CodeGenObjCGNU concrete  
> subclass for the GNU runtime.

That's good idea.

Few  nitpicks:
- I'd shorten class names by replacing "CodeGen" with CG.
- Please use vertical spaces (empty lines) appropriately to make code  
easy to read.
- Stay within 80 cols.


>  Currently, this only handles message sends (and assumes an id  
> return type, which needs fixing later).  It's also completely  
> unoptimised as yet, and does slightly silly things like look up the  
> selector every message send, rather than caching it after the first  
> lookup

OK.

> (I'm not sure if this should be done here or if a later pass should  
> know that sel_get_uid is a constant function and do the caching as  
> an optimisation).


> I am trying to keep the CodeGenObjC* classes free from clang- 
> specific classes because I intend to use them with a Smalltalk and  
> Self compiler (which won't use the clang AST representations) as well.
>
> David
>
> P.S. I've not used C++ for almost five years and not looked at the  
> llvm/clang sources before Sunday, so I expect lots of comments.

+  //TODO: Make this selectable at runtime
+  ObjC = ObjCDefaultRuntime(this);
+}

Rename ObjC as Runtime and propagate it down stream (e.g. see how  
Builder is propagated)

+Value *CodeGenObjCGNU::generateMessageSend(llvm::LLVMFoldingBuilder &B,
+                                           llvm::Value * receiver,
+                                           std::string &selector,
+                                           llvm::Value** ArgV,
+                                           size_t ArgC) {

Consistently use capital letter, e.g. Receive and Selector.
Please use Builder instead of just B.

+  const llvm::Type *selType =  
llvm::PointerType::get((llvm::Type*)llvm::StructType::get(str2), 0);

Avoid (llvm::Type*) casts. Instead
    const llvm::Type *STy = llvm::StructType::get(str2);
    const llvm::Type *SelType = llvm::PointerType::get(STy, 0);

Be consistent in naming local variables, (str2, FT, Idx0,  
index_vector ..)


+      } else if (dyn_cast<ObjCClassDecl>(D)){
+        //Forward declaration.  Only used for type checking.
+      } else if (dyn_cast<ObjCProtocolDecl>(D)){
+        // TODO: Generate Protocol object.
+      } else if (dyn_cast<ObjCCategoryDecl>(D)){
+        //Only used for typechecking.
+      } else if (dyn_cast<ObjCCategoryImplDecl>(D)){
+        // TODO: Generate methods, attach to class structure
+      } else if (dyn_cast<ObjCImplementationDecl>(D)){
+        // TODO: Generate methods, attach to class structure
+      } else if (dyn_cast<ObjCInterfaceDecl>(D)){
+        // TODO: Set up class structure
+      } else if (dyn_cast<ObjCMethodDecl>(D)){
+        // TODO: Emit method, add method pointer to class structure.

We want to use assert() or Assert1()  to emit an explicit "not  
implemented ... " message instead of silent TODOs.

Overall, very good start.
-
Devang