After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 350606 - Location of G_GNUC_INTERNAL breaks Solaris build
Location of G_GNUC_INTERNAL breaks Solaris build
Status: RESOLVED FIXED
Product: gtk-engines
Classification: Deprecated
Component: general
2.7.x
Other Solaris
: Normal normal
: ---
Assigned To: gtk-engines maintainers
gtk-engines maintainers
Depends on:
Blocks:
 
 
Reported: 2006-08-09 14:55 UTC by Damien Carbery
Modified: 2006-08-09 21:25 UTC
See Also:
GNOME target: ---
GNOME version: 2.15/2.16


Attachments
Move G_GNUC_INTERNAL macro to the start of the line (116.69 KB, patch)
2006-08-09 14:56 UTC, Damien Carbery
none Details | Review

Description Damien Carbery 2006-08-09 14:55:30 UTC
gtk-engines makes extensive use of the G_GNUC_INTERNAL macro.
For the Solaris forte compiler this must be placed at the start of the line.

Can you please apply the attached patch this does this 650 times for various gtk-engines components.
This will not affect gcc - it does not care where the macro is placed.
Comment 1 Damien Carbery 2006-08-09 14:56:13 UTC
Created attachment 70554 [details] [review]
Move G_GNUC_INTERNAL macro to the start of the line
Comment 2 Andrew Johnson 2006-08-09 15:22:36 UTC
Hum Position matters. 

Before I go ahead and commit this, does this affect other compilers? Are there any compilers we might need worry about that does things yet different?

Since I don't have much non-gcc experience I just want to make sure we don't have to deal with, this breaks one compiler but fixes another.
Comment 3 Brian Cameron 2006-08-09 18:03:07 UTC
No, this is a safe change.  The macro as defined will only ever be available if the GCC compiler is used, and only works with the GCC compiler.  The GCC compiler does not care about the position of the macro (whether it comes before or after) the function definition).  SunPro compiler is more picky and requires it at the beginning.

We at Sun would like to modify this macro so it works like the way they do it in the Xorg code.  They use these macros, for example:

#if defined(__GNUC__) && ((__GNUC__ * 100 + __GNUC_MINOR__) >= 303)
# define _X_EXPORT      __attribute__((visibility("default")))
# define _X_HIDDEN      __attribute__((visibility("hidden")))
# define _X_INTERNAL    __attribute__((visibility("internal")))
#elif defined(__SUNPRO_C) && (__SUNPRO_C >= 0x550)
# define _X_EXPORT      __global
# define _X_HIDDEN      __hidden
# define _X_INTERNAL    __hidden
#else /* not gcc >= 3.3 and not Sun Studio >= 8 */
# define _X_EXPORT
# define _X_HIDDEN
# define _X_INTERNAL
#endif 

So since the Xserver makes it more of a standard to support this at the beginning, it would probably be good if GNOME were consistant with that usage.
Note if you grep through the GNOME code, the current G_GNUC_INTERNAL macro is used a bit willy-nilly.  Some modules put it at the beginning and some at the end.  We're just suggesting the beginning is a better place for cross-compiler compatibility.
Comment 4 Brian Cameron 2006-08-09 18:07:04 UTC
Also note how cairo dealt with this same issue, it may be better to use a 
private macro like they do rather than using a macro that has GNUC in its name.  Just because a macro with GNUC in its name doesn't well imply cross-compiler support, and we should probably get away from using it really.

http://gitweb.freedesktop.org/?p=cairo;a=blobdiff;h=9d1c789d2209aab3f969cc1e6baef5a997826e85;hp=447c1ac0bb8fba938495c26abefaecb3e1f87f78;hb=04757a3aa8deeff3265719ebe01b021638990ec6;f=src/cairoint.h
Comment 5 Andrew Johnson 2006-08-09 21:25:49 UTC
Got it. Understood. This was easy enough to do. 

I am closing as I have applied this patch and fixed a few that it missed, and renamed them too a new GE_INTERNAL.