Hi Niels,

A few minor style points:

- Never use the NS prefix for anything outside of Foundation or  
AppKit; creating new NS* functions is very likely to cause future  
breakage.

- Always declare functions that aren't used outside of a compilation  
unit as static to avoid undue namespace pollution (this has a few  
small speed benefits, but most importantly it prevents conflicts when  
two files declare the same private function).

- Don't duplicate code; there are existing methods in  
EtoileFoundation's NSObject additions and in ObjectiveC2.framework for  
finding the type and address of a given ivar.

- If you need to expose private GNUstep ivars for any reason, declare  
a category on the object that returns them.  This will break at  
compile time if the ivars are removed in future versions; your  
implementation will fail at runtime (and you have no error checking  
for this case).

Why are you allocating the BOOL on the heap?  Just do the same thing  
as the +map example:

BOOL result = NO;
[inv getReturnValue: &result];

For safety, you should use a long long rather than a BOOL, like this:

long long result = 0;
[inv getReturnValue: &result];

This is because the comparison method may return any integer type up  
to 64-bits.

Now for the difficult bit.  You can't actually implement this method  
like this.  The problem comes from the fact that you have to return a  
value.  The compiler has to know the return type of a method in order  
to handle it correctly.  The return type of -isEqualToString: is a  
BOOL (i.e. an 8-bit quantity), and so it will allocate an 8-bit space  
on the stack.  When you try to return an object pointer (a 32- or 64- 
bit quantity), it will either be truncated, or will trample on some  
other on-stack values.  Either of these is bad.  The reason for your  
compiler warning is that it is then reading the BOOL (either from a  
register or the stack, depending on your architecture) and then sign- 
extending it to give a pointer, which is pretty much guaranteed to  
always give an invalid memory address (on most modern architectures,  
at least the bottom 4KB of memory is mapped no-access to catch NULL  
pointer dereferencing).

One alternative might be to make the filter proxy an NSArray  
subclass.  This is quite easy:

1) Make the array that you would have returned into an ivar.
2) Implement -count and -objectAtIndex: to call the corresponding  
methods on the ivar.
3) Implement -copy and -mutableCopy to call the corresponding methods  
on the ivar.

The third step means that users can get at the original array if they  
need to.  This isn't ideal, but would have the advantage of actually  
working.  Alternatively you could add another method for getting the  
result so you'd do something like:

NSArray *filteredArray = [[[myArray filter] isEqualTo: someObject]  
result];

Probably the best option, however, is to make this only applicable to  
mutable arrays, and have it modify the original receiver.  Then you'd  
use it like this:

[[myArray filter] someCondition: wibble];

This better matches the OpenStep naming conventions too.  The existing  
-map method should really be renamed -mappedArray and -map should  
instead only work on mutable arrays and modify the array.

David

On 13 Jun 2009, at 07:01, Niels Grewe wrote:

> On Thu, Jun 11, 2009 at 04:04:47PM +0100, David Chisnall wrote:
>> Patches welcome for other higher-order messaging functions (e.g. the
>> same thing on NSSet, fold and similar operations)
>
> I'd really like to have them as well and in a surge of major  
> ignorance I
> tried to implement -filter on NSArray yesterday, which turns out to be
> less than trivial. I originally thought one get away with minor
> modifications to the mapping code, but due to the nature of  
> NSInvocation
> that is not the case.
>
> An example: Suppose you are trying to send an -isEqualToString:  
> message
> to all string objects in an array, the second-chance dispatch  
> mechanism
> contructs an invocation based on the method signature of that method.
> This way, it is set up for a BOOL return type, but what is returned
> after forwarding the invocation to the members of the array is  
> really an
> object.
>
> I'm now doing ugly stuff to the runtime by manually modifying the  
> method
> signatures and invocations, but it's still not working. I stepped
> through the whole process with gdb and the NSInvocation/ 
> GSFFIInvocation
> code seems to handle the result flawlessly. (I have no idea what is  
> going
> on inside libffi, though.)
>
> I'm now highly suspecting that the compiler is to "blame" for using  
> the
> type information from -isEqualToString:. It's barking at me that I'm
> trying to cast an integer type (BOOL?) to a differenty sized  
> pointer, so
> perhaps I should assume this to fail. FWIW it the pointer I'm trying  
> to
> return seems to be mangled somewhere: The return value is always nil.
> Unfortunately, I know next to nothing about compilers and am  
> thoroughly
> at a loss now.
>
> I have attached my (admittedly, rough) implementation and sample  
> code to
> demonstrate the problem. I understand that you probably wont have the
> time to look at this, but I highly appreciate any pointers on where to
> look next.
>
> kind regards,
>
>
> Niels
> <sample.m><NSArray 
> +filter.m>_______________________________________________
> Etoile-dev mailing list
> [email protected]
> https://mail.gna.org/listinfo/etoile-dev


_______________________________________________
Etoile-dev mailing list
[email protected]
https://mail.gna.org/listinfo/etoile-dev

Reply via email to