Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stage 8 of clang compiler warning patches. #30

Merged
merged 2 commits into from
Nov 4, 2015

Conversation

ethoms
Copy link
Contributor

@ethoms ethoms commented Oct 24, 2015

Stage 8: CAUTION

I have scrapped my original patches that descretely fix build errors on FreeBSD (or rather libobjc2). SOPE is riddled with messy runtime compatability definitions, dating back a long time. I'm talking about 'sel_eq', 'sel_get_name', 'objc_malloc', 'objc_free' etc. There was lots of ugly, inconsistent and nonsensical #if's and #ifdefs for all the different brands and ages of Objective C runtimes.

So, I cleaned up all this stuff in the common.h header files, and used only the modern calls throughout the implementation code. But of course, I was worried about the compatability with the various platforms and older runtimes. So I installed RHEL 5.11 in VirtualBox and managed to build SOPE from my github fork. It worked like a charm!

So, the GNUstep Runtime (libobjc2) and the GCC Runtime (libobjc) >= 4.1.2 build successfully with all my changes up to Stage 8. But I have not tested on any Apple/NeXT Runtimes (Mac OS X), since I don't have access to OS X. According to David Chisnall, it should be OK to use the same modern calls that work in libobjc2.

If the support needs to go back further, and the modern calls are not OK. Then I can add back some much neater, consolidated compatability definitions. I suggest each common.h header file include a project wide header file called runtime_compat.h. Then the runtime specific substitutions could all go in one place.

There is potential for much more, similar, cleanup in SOPE.

register unsigned char *byteBuffer = _buf;

for (len = result - 1; len >= 0; len--, byteBuffer++) {
for (NSInteger len = result - 1; len >= 0; len--, byteBuffer++) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You overwrite the changes here that Ludovic did with 3778286 ((fix) c99 fixes). That breaks my build with gcc-4.2.1, libobjc2, on my macppc.

@ethoms
Copy link
Contributor Author

ethoms commented Oct 26, 2015

That worries me! That means I didn't branch properly before I made the Stage 8 pull request. Each Stage should be branched from upstream/master before I commit and make a PR. I'll look into this, I may have to redo Stage pull request.

@buzzdeee
Copy link

Besides the comment above:
Tested this together with the other stages:
OpenBSD -current

  • amd64, clang-3.5, libobjc2-1.8
  • i386, clang-3.5, libobjc2-1.8
  • macppc, gcc-4.2.1, libobjc2-1.8
    No issues so far, besides my comment on the stage8 fix

I should mention, on all test boxes I run against a local postgresql 9.4.4, and against LDAP authentication.

@ethoms
Copy link
Contributor Author

ethoms commented Oct 26, 2015

Is there an easy way to change the existing commit/PR, and remove the NGByteCountStream.m changes? Otherwise I'll have to close the PR, re-branch from upstream/master, re-apply my changes and redo the PR.

@ethoms
Copy link
Contributor Author

ethoms commented Oct 26, 2015

OK, github allows us to do this in an editor, NICE!

So, I've cleared that mistake. But I wonder how to get those changes back into my local copy.

extrafu added a commit that referenced this pull request Nov 4, 2015
Stage 8 of clang compiler warning patches.
@extrafu extrafu merged commit 5fc138f into Alinto:master Nov 4, 2015
@ethoms ethoms deleted the clang-warnings-stage8 branch November 27, 2015 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants