[cfe-dev] Objective-C top-level constructs code generation

[Chris Lattner]

On May 28, 2008, at 1:26 PM, David Chisnall wrote:
>> +  std::map<std::string, llvm::Constant*> ExistingProtocols;
>> +  typedef std::pair<std::string, std::string> TypedSelector;
>> +  std::map<TypedSelector, llvm::GlobalAlias*> TypedSelectors;
>> +  std::map<std::string, llvm::GlobalAlias*> UntypedSelectors;
>>
>> std::map's from std::string are really inefficient.  Can you use
>> StringMap for these?
>
> Done for the two with string keys.  I'm happy to accept suggestions  
> for a more efficient way of storing typed selectors (although clang  
> currently never uses that code path, so the inefficiency isn't  
> important).

If TypedSelectors are never used?  Why have them?

One simple way to do it with stringmap is to insert the string with a  
\0 or some other sentinel between the two pieces.

>> +void CGObjCGNU::GenerateProtocol(const char *ProtocolName,
>>
>> It would be nice to add a block comment above each of these methods
>> with a C struct (in the comment) that describes the thing that you  
>> are
>> generating.  This would make it easier to follow the code.
>
> I don't want to copy the structs directly from the runtime headers,  
> since the header is GPL'd (with a special exemption for code  
> compiled with GCC, but not for code compiled with LLVM) and,  
> although you can't copyright an interface, you can copyright a  
> representation of an interface (see AT&T Vs UCB).  I've added a  
> comment at the top of the file pointing people in the right  
> direction for finding the structures.

Fair enough.  However, you don't have to copy and paste the structs,  
just rewrite them based on your knowledge of what they are, for  
example looking at the code that emits the LLVM IR and naming the  
fields how you wish.

+++ lib/CodeGen/CGObjCGNU.cpp	(working copy)
+#include <cstdarg>

Is this needed?



+const static int RuntimeVersion = 8;
+static llvm::Constant *ProtocolVersion =
+  llvm::ConstantInt::get(llvm::Type::Int32Ty, 2);

RuntimeVersion is ok, but ProtocolVersion still requires a static  
ctor.  Please remove.  It is also slightly more idiomatic to use  
"static const int" instead of "const static int", but it doesn't make  
that much of a difference.

+static std::string SymbolNameForMethod(const std::string &ClassName,  
const
+    std::string CategoryName, const std::string &MethodName, bool  
isClassMethod)

CategoryName is still passed by copy

+  if (llvm::Constant *CName =  
llvm::dyn_cast<llvm::Constant>(SelName)) {

I think you can just use dyn_cast instead of llvm::dyn_cast.  If not,  
I suggest adding a 'using llvm::dyn_cast;' to the top of the cpp file.

+      llvm::GlobalAlias *Sel = new  
llvm::GlobalAlias(llvm::PointerType::getUnqual(SelectorTy),
+          llvm::GlobalValue::InternalLinkage,  
".objc_untyped_selector_alias", NULL,
+          &TheModule);

80 cols, also elsewhere.


Otherwise, looks great.  Please commit with these updates.  Thanks  
David!

-Chris