Re: [PATCH 00/13] Patches for handling property attributes correctly in objc.

[David Chisnall]

Hi,

On 27 Feb 2013, at 23:44, Jean-Charles BERTIN <address@hidden> wrote:

> OK I admit I did that too quickly but I have push my changes on github a
> long time ago...
> What is the point to make a github account if nobody reviews the pull
> requests? Since I saw no feedbacks from github I decided to push my
> patches on the mailing list: I did that by pushing every single git
> commit as a patch to discuss them one-by-one. I'm ok with the reject of
> some of them since I'm really new with GNUstep (but not with objc on
> MacOS X).

The github mirror is just a mirror.  It is read-only.

> The main goal of this stuff is to fix compatibilty problems I found
> when I tried to compile a code which works flawlessly on MacOS but
> fails with GNUstep. I make a complete test case in
> Test/PropertyAttributeTest2.m which can be compiled on both GNUstep and
> MacOS (clang -o PropertyAttributeTest2 -framework Foundation
> Test/PropertyAttributeTest2.m should do it on MacOS)
> If you make a try of this test case with a vanilla GNUstep you will see
> that every test fail on GNUstep but work on MacOS.
> That's why I tried to fix that without breaking existing code so I
> introduced the objc_property_clean_abi in clang and bumped the GNUstep
> runtime version (I really missed that GNUstep runtime 1.7 was not yet
> released)

Actually, about half of the tests in that file were working already.  It's a 
really great test case though, thank you.  
They now all pass, with clang r176254 and libobjc2 r36205.

> I really apologize if I make some mistakes but I don't really understand
> how the mkstemp stuff can work: the first time you invoke mkstemp(),
> tmpPattern is modified and then contains a unique random path that you
> use for mapping the first trampoline block. The next time, mkstemp()
> will return the same unique random path that you use for mapping the
> next trampoline block. This works because 1) the first time you unlink
> the file that was opened 2) the next time mkstemp() will return an
> another file descriptor that will map an another trampoline block. Am I
> right?

Yes

> Another question: which patch to you talk about pragmas for fixing a
> warning?

I don't remember.  You were silencing a cast from int to id by turning off the 
warning, when a (NSInteger) cast in the middle would have done the job better.

> I hope you will take a deeper look at my commits because I really think
> that property attributes stuff is broken in regards to MacOS.

I've taken a closer look at the test case, and now fixed all of the bugs that 
it indicates.

I'd welcome more people to be familiar with the runtime's internals, but please 
be aware that maintaining ABI compatibility is VERY important for the runtime.  
It is completely unacceptable for a new version of the runtime not to work with 
code targeting earlier version.  If you have more questions, please let me know.

David

-- Sent from my Cray X1