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

Attachment: clang.diff
Description: Binary data


_______________________________________________
cfe-dev mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

Reply via email to