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 :)
--- 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.
+#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.
+using llvm::Value;
Headers shouldn't have "using", this pollutes the namespace of the
file that includes it.
+ //TODO: Make this selectable at runtime
+ Runtime = ObjCDefaultRuntime(M);
+}
This object needs to be deleted in ~CodeGenModule().
+ 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.
+ // 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);
Otherwise, looks great! Thanks for working on this David!
-Chris
_______________________________________________
cfe-dev mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
