[PATCH] [NS] Fix compilation with clang on OSX

[PATCH] [NS] Fix compilation with clang on OSX

[İsmail Dönmez]

[-- Attachment #1.1: Type: text/plain, Size: 136 bytes --]

Hi;

Attached is a patch fixing emacs compilation with clang on OSX. Just some
minor problems.

Please review & apply.

Regards,
ismail

[-- Attachment #1.2: Type: text/html, Size: 790 bytes --]

[-- Attachment #2: 0001-Fix-compilation-problems-with-clang-add-explicit-ret.patch --]
[-- Type: application/octet-stream, Size: 1571 bytes --]

From ccdc0a4851158cf1ed7908569459e221997bd1ab Mon Sep 17 00:00:00 2001
From: Ismail Donmez <ismail@namtrac.org>
Date: Thu, 4 Nov 2010 17:53:49 +0200
Subject: [PATCH] Fix compilation problems with clang, add explicit returns to non-void functions, mark void functions as such

---
 src/nsfont.m  |    2 +-
 src/nsimage.m |    2 +-
 src/nsterm.m  |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/nsfont.m b/src/nsfont.m
index 1159867..e1450c5 100644
--- a/src/nsfont.m
+++ b/src/nsfont.m
@@ -1280,7 +1280,7 @@ nsfont_draw (struct glyph_string *s, int from, int to, int x, int y,
       }
 
     CGContextRestoreGState (gcontext);
-    return;
+    return 0;
   }
 #endif  /* NS_IMPL_COCOA */
 
diff --git a/src/nsimage.m b/src/nsimage.m
index a42950d..efaab4b 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -327,7 +327,7 @@ static EmacsImage *ImageList = nil;
 
 /* Set color for a bitmap image (see initFromSkipXBM).  Note that the alpha
    is used as a mask, so we just memset the entire array. */
-- setXBMColor: (NSColor *)color
+- (void)setXBMColor: (NSColor *)color
 {
   NSSize s = [self size];
   int len = (int) s.width * s.height;
diff --git a/src/nsterm.m b/src/nsterm.m
index f11c477..2fd82a0 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -5436,7 +5436,7 @@ ns_term_shutdown (int sig)
   NSTRACE (performDragOperation);
 
   if (!emacs_event)
-    return;
+    return NO;
 
   position = [self convertPoint: [sender draggingLocation] fromView: nil];
   x = lrint (position.x);  y = lrint (position.y);
-- 
1.7.3.2.145.g7ebee

Re: [PATCH] [NS] Fix compilation with clang on OSX

[David Reitter]

On Nov 4, 2010, at 12:15 PM, İsmail Dönmez wrote:
> Attached is a patch fixing emacs compilation with clang on OSX. Just some minor problems.

These are good, even for the non-clang compilation case.

In nsfont_draw(), don't you want to return (to-from) rather than 0?

Have you obtained significant performance benefits from compiling with clang?

Re: [PATCH] [NS] Fix compilation with clang on OSX

[İsmail Dönmez]

Hi;

On Thu, Nov 4, 2010 at 7:52 PM, David Reitter <david.reitter@gmail.com> wrote:
>
> On Nov 4, 2010, at 12:15 PM, İsmail Dönmez wrote:
> > Attached is a patch fixing emacs compilation with clang on OSX. Just some minor problems.
>
> These are good, even for the non-clang compilation case.

Indeed!

> In nsfont_draw(), don't you want to return (to-from) rather than 0?

Currently it just returns which is equivalent to 0 in gcc case, so I
didn't want to change that.

> Have you obtained significant performance benefits from compiling with clang?

Compilation is faster, warning diagnostics are better, speed wise it
seems equal to my eyes :)

Regards,
ismail

Re: [PATCH] [NS] Fix compilation with clang on OSX

[Adrian Robert]

İsmail Dönmez <ismail <at> namtrac.org> writes:

> Attached is a patch fixing emacs compilation with clang on OSX. Just some
minor problems.


Thanks, I've applied a modified version to the trunk.

-Adrian

Re: [PATCH] [NS] Fix compilation with clang on OSX

[Glenn Morris]

Adrian Robert wrote:

> Thanks, I've applied a modified version to the trunk.

Please try not to use things like "based on a patch by X" in
ChangeLogs, since it is hard to figure out who the actual author of
the change really is.

Two alternatives:

1) Put both your names as the author (and mark with "tiny change" in
this case).

2) Make the original change in his name (marked "tiny change"); then
apply your correction as a separate change.

(Tiny change is because this change is small and we don't have
assignment papers.)

Re: [PATCH] [NS] Fix compilation with clang on OSX

[Adrian Robert]

On 2010/11/05, at 2:04, Glenn Morris wrote:

> 1) Put both your names as the author (and mark with "tiny change" in
> this case).


Like this?

> 2010-11-04  Adrian Robert  <Adrian.B.Robert@gmail.com>
> 2010-11-04  Ismail Donmez  <ismail@namtrac.org>  (tiny change)
> 
> 	* nsfont.m (nsfont_draw)
> 	* nsimage.m (EmacsImage-setXBMColor:)
> 	* nsterm.m (EmacsView-performDragOperation:): Correct empty return
> 	statements.


It seems to be more ambiguous than putting one "communicating author" and an attribution.



> 2) Make the original change in his name (marked "tiny change"); then
> apply your correction as a separate change.


Like this:

> 2010-11-04  Adrian Robert  <Adrian.B.Robert@gmail.com>
> 
> 	* nsfont.m (nsfont_draw):  Correct previous patch to return
> 	correct value.
> 	* nsimage.m (EmacsImage-setXBMColor:): Correct previous patch:
> 	don't change the method signature, change the return.
> 
> 2010-11-04  Ismail Donmez  <ismail@namtrac.org>  (tiny change)
> 
> 	* nsfont.m (nsfont_draw)
> 	* nsimage.m (EmacsImage-setXBMColor:)
> 	* nsterm.m (EmacsView-performDragOperation:): Correct empty return.


If people would really prefer this I can do it in the future.  (Or hassle the patch submitter to redo their patch, I suppose.)  The reason I haven't before is when you are dealing with a "tiny change" in the first place, going into this kind of detail seems excessive.



-Adrian