Re: libobjc2 release?

[David Chisnall]

On 13 Mar 2010, at 20:38, Richard Frith-Macdonald wrote:

> 
> On 13 Mar 2010, at 13:48, David Chisnall wrote:
> 
>> @interface NSOperation : NSObject
>> {
>> #if>GS_NONFRAGILE
>> #  if>--defined(GS_NSOperation_IVARS)
>> @public GS_NSOperation_IVARS
>> #  endif
>> #else
>> @private id _internal;
>> #endif
>> }
>> 
>> This is 100% nonsense.  For compatibility with both ABIs, please copy what I 
>> did in NSCache.h.  Looking at that file, it appears that you have 
>> subsequently changed the code there to make it break when you subclass 
>> NSCache and compile with GCC or clang with the fragile ABI too.
> 
> I'm not sure what 100% nonsense  means ... looks like it's doing exactly 
> what's needed as I understand it ...
> 
> If we are building for use without the nonfragile abi, then we hide all the 
> instance variables behind a single value '_internal' ... this is the common 
> case (ie the setup almost everyone currently has).
> If we are building for use *with* the nonfragile abi, then either
> a. we are building the class implementation itsself, and all instance 
> variables are visible or
> b. we are building some code which uses the class ... and no  instance 
> variables are visible
> 
> I think what was really missing was a check to ensure that we don't 
> accidentally try to work without the nonfragile abi using a copy of the base 
> library built to work *with* it.
> I've added a bit of preprocessor stuff which (unless I made a mistake) ought 
> to check that.

The non-fragile ABI is specifically designed for graceful fallback to fragile 
behaviour in cases where the superclass is compiled with the non-fragile ABI 
but the subclass isn't.  

This means that the correct thing to do is hide private instance variables 
entirely when compiling with the non-fragile ABI but make them visible (but 
@private) when compiling with the fragile ABI.  

Your macros are ignoring the example ones that I created and intentionally 
breaking GCC when you you compile -base with the non-fragile ABI.  

I have no idea why you would consider this to be a good thing.

>> Unrelated, but the test here is very unreliable as it depends on the CPU 
>> load and scheduler interaction.  It works correctly for me around 50% of the 
>> time, but the "thread exit occurs after five seconds" test fails at other 
>> times.  
> 
> I'll have to check that ... unless you already have a better test?

Just sleeping an extra second would probably be good.

>> The GNUstep test suite would be a lot more helpful if it wasn't full of 
>> tests that always (or commonly) fail.
> 
> It isn't.

Really?  I have never managed to run it, even with a fresh checkout, without a 
number of failures.  Perhaps they work on GNU/Linux.

>> Marking these as expected failures would, at least, make tracking 
>> regressions easier.  I rarely bother running it, because I never know which 
>> of the 20+ failures are potentially the result of something that I've done, 
>> and which are just broken tests.
> 
> 
> Here's a run:
> 
> --- Running tests in base ---
> 
> base/Functions/runtime.m:
> FAIL: SubClass1 has one protocol
> 
> base/NSException/basic.m:
> FAIL: raises exception in description
> FAIL: working callStackSymbols
> : Uncaught exception NSGenericException, reason: Terminate
> FAIL: base/NSException/basic.m
> 
> base/NSLock/doubleLocking.m:
> FAIL: Recursive lock with NSLock deadlocks ... this is not a real test, just 
> a reminder that recursively locking should deadlock the thread after printing 
> a diagnostic message
> FAIL: Recursive lock with NSConditionLock deadlocks ... this is not a real 
> test, just a reminder that recursively locking should deadlock the thread 
> after printing a diagnostic message
> 
> base/NSObject/test02.m:
> FAIL: +conformsToProtocol returns YES on a protocol implemented in a category
> FAIL: -conformsToProtocol returns YES on a protocol implemented in a category
> 
> base/NSURL/basic.m:
> FAIL: Can load a page from www.w3.org
> FAIL: Status of load is 200 for www.w3.org
> FAIL: Status of load is 404 for www.w3.org/silly-file-name
> 
>      1 
>    259 COMPLETED
>     11 FAIL
>   5219 PASS
> 
> That's 11 failures in 5230 tests ... even if the 11 were all broken tests, it 
> would seem rather irrational to not run the testsuite with a ratio of working 
> test so broken of 52350:11 just because you aren't sure if the 11 are real 
> failures.  It would make hugely more sense to fix any broken tests!

I get 26 test failures currently, compiling base with GCC and using the fragile 
ABI.  I don't think I've ever seen fewer than 20, with a clean checkout and 
stock libobjc.  I presumed that they were known failures, so I ignored them.  
When I ran the test suite again and got a similar number of failures, I had no 
way of knowing if they were regressions or old failures.  

> looking at the failures:
> 
> 1, in runtime.m the count of protocols is a libobjc2 failure I already 
> flagged up to you... a real issue, not a broken test.

This is a clang bug.  A copy-and-paste error meant that categories were 
declaring all of the protocols from the class interface, not the category 
interface.  I've fixed it in trunk, and sent Chris Lattner an email asking if I 
can sneak the fix into the 2.7 release branch.  The base/NSObject/test02.m test 
failures were almost certainly caused by this too.

David

-- Sent from my Difference Engine