[cfe-dev] New diff

[Chris Lattner]

<moving this to cfe-dev, which should have been cc'd the whole time>

On Jun 28, 2008, at 7:41 AM, David Chisnall wrote:
> On 27 Jun 2008, at 23:33, Chris Lattner wrote:
>> On Jun 26, 2008, at 9:22 AM, David Chisnall wrote:
>>
>>> Fixed a bug where I was returning the wrong thing while generating  
>>> metaclass structures (almost the same as the last diff).
>>
>> This patch doesn't apply.
>>
>
> Here's a new version.  It pulls out most of the cases where the  
> value from an llvm::Constant is taken in CodeGen and passes in the  
> initialisers instead.

Now you're copying strings around all over the place for no reason.   
Converting things to a vector<std::string> does a bunch of string  
copies.  Why do we want this?

> I've also changed it so that the Selector passed to the the methods  
> for generating message sends is a selector, rather than the name of  
> a selector.  This should be previously obtained by GetSelector.   
> There are now two versions of this, one taking a pair of char*s and  
> one taking a pair of llvm::Value*s as arguments.

I don't understand where you're going with this.  You've taken all the  
nice simple APIs that take Selectors and pass Value*'s around.  You  
are inverting the logic of the code generator to avoid passing in  
clang data structures.

Further, I don't see why the ObjC codegen stuff shouldn't pull in  
CodeGenModule.  It seems that you're again inverting control flow here  
to eliminate a very reasonable dependency.  Because of this, you've  
reinvented things (like CGObjCGNU::MakeConstantString), and introduced  
problems (e.g. MakeConstantString doesn't unique the strings like  
CGM's does).


> There are now two versions of this, one taking a pair of char*s and  
> one taking a pair of llvm::Value*s as arguments.  The first returns  
> a statically-bound selector, the latter returns a dynamically-bound  
> selector.  This seems cleaner and also makes it easier to support  
> languages where message dispatch can be performed with an extra  
> layer of indirection (I'm not doing this at the moment but I know  
> people who want to use this code for JavaScript and Io  
> implementations and it will help them).  Currently everything in  
> clang uses untyped selectors, but I plan on using typed selectors  
> when available in the future (i.e. generate a selector type string  
> from the types passed to the selector).  This will allow the Étoilé  
> runtime (which uses selector types for dispatch, where available) to  
> either throw an exception or perform auto [un]boxing at message-send  
> time, rather than just corrupt the stack (as the GNU and NeXT/Apple  
> runtimes do).


I understand (now) that you are trying to reuse the clang codegen in  
the context of your smalltalk implementation.  While this is an  
interesting goal, this is *not* an acceptable reason to make the clang  
ObjC codegen code inefficient and weird.  I'm somewhat ok with you  
adding codepaths that are not used by clang (and are thus dead) as  
long as they are not huge, but making codepaths clang *does* use  
inefficient and inelegant is not acceptable.


    return Builder.CreateCall(imp, Args.begin(), Args.end());
  }
-
+#include "llvm/ValueSymbolTable.h"
  /// Generates a MethodList.  Used in construction of a objc_class and

Please don't include things in the middle of .cpp files.

-Chris

-------------- next part --------------
A non-text attachment was scrubbed...
Name: objc.diff
Type: application/octet-stream
Size: 48284 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080628/8d1450c5/attachment.obj>
-------------- next part --------------