[cfe-dev] Objective-C Message Send Generation

[David Chisnall]

On 29 Feb 2008, at 17:22, Chris Lattner wrote:

>
> On Feb 29, 2008, at 7:16 AM, David Chisnall wrote:
>
>> 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.
>
> Ok!  Thanks again for working on this by the way.
>
> Here are some more picky details :)

Keep them coming - I'm still trying to remember how C++ works...

> --- CGObjCRuntime.h	(revision 0)
> +++ CGObjCRuntime.h	(revision 0)
> @@ -0,0 +1,46 @@
>
> +#ifndef __CODEGENOBJC_H_INCLUDED__
> +#define __CODEGENOBJC_H_INCLUDED__
>
> Please use a name that isn't in the implementation namespace, for  
> example CLANG_CODEGEN_OBCJRUNTIME_H like the other headers.

Fixed.

> +#include "clang/AST/AST.h"
> +#include "clang/AST/Expr.h"
> +
> +#include "llvm/Constants.h"
> +#include "llvm/Function.h"
> +#include "llvm/GlobalVariable.h"
> +#include "llvm/Intrinsics.h"
> +#include "llvm/Support/Compiler.h"
> +#include "llvm/Support/LLVMBuilder.h"
> +#include "llvm/Module.h"
>
> Please don't #include headers redundantly (Module.h pulls in  
> function.h, for example).  Also, please use forward definitions of  
> classes to avoid #including as much as possible.  In this case, you  
> should be able to fwd declare all the llvm classes you use.

I think that's fixed now.  Let me know if I've missed any.

> +using llvm::Value;
>
> Headers shouldn't have "using", this pollutes the namespace of the  
> file that includes it.

Ooops.  Fixed.

> +  //TODO: Make this selectable at runtime
> +  Runtime = ObjCDefaultRuntime(M);
> +}
>
> This object needs to be deleted in ~CodeGenModule().

This destructor didn't seem to exist.  I've created it, but let me  
know if I've put it in the wrong place...

> +  Function *SelFunction = TheModule.getFunction("sel_get_uid");
> +  // If we haven't got the selector lookup function, look it up now.
> +  // TODO: Factor this out and use it to implement @selector() too.
> +  if(SelFunction == 0) {
> +    std::vector<const llvm::Type*> Str(1,
> ....
> +  }
>
>
> This code can be simplified through the use of the  
> Module::getOrInsertFunction method.  Something like this should work:
>
> PtrToInt8Ty = llvm::PointerType::getUnqual(llvm::Type::Int8Ty);
> llvm::Constant *SelFunction =  
> TheModule.getOrInsertFunction("sel_get_uid", PtrToInt8Ty, NULL);
>
> A similar approach can simplify creation of the objc_msg_lookup  
> function.

Ah, I thought there was probably a simpler way of doing this (not  
quite like that - the return type should be a selector).  Since that  
does implicit type coercion of the function it will also make it  
easier to handle the return types.

> +  // Call the method
> +  for(size_t i=0 ; i<ArgC ; i++) {
> +    lookupArgs.push_back(ArgV[i]);
> +  }
>
> This loop can be replaced with:
>
> lookupArgs.insert(lookupArgs.end(), ArgV, ArgV+ArgC);

Ah yes.  Fixed.

Here's a new version of the diff.  I've also tidied up a few of the  
type definitions.  Let me know if I've missed anything important...

David

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