[cfe-dev] Objective-C Message Send Generation

[David Chisnall]

On 28 Feb 2008, at 18:19, Devang Patel wrote:

> Hi David,
>
> On Feb 28, 2008, at 7:29 AM, David Chisnall wrote:
>
>> 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.

Done.

> - Please use vertical spaces (empty lines) appropriately to make code
> easy to read.

I've separated semantic blocks with some white space.

> - Stay within 80 cols.
>
>
> +  //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)

Done, I think.  Please check I have done the propagation correctly.  I  
am not sure if it should be propagated via CodeGenFunction?

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

Ooops.  I switched to using the LLVM style part way through and didn't  
update everything.  I think I've fixed that now.


> +  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 ..)

That's what happens when you try to switch coding styles part way  
though...
Should be fixed now.

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

I'll keep these in my local copy because they let me compile simple  
Objective-C programs without hitting aborts, and add the missing bits,  
but I've removed them from the diff.  I've also removed the inclusion  
of iostream that I was using for debugging and shouldn't be there  
anymore.

David

-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang.diff
Type: application/octet-stream
Size: 11307 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080229/0ea1a5e2/attachment.obj>
-------------- next part --------------