Created attachment 41551 [details] [review] Hacky workaround for buggy(?) cpp When building pixman with sun studio 12.2 it warns like: CC pixman-fast-path.lo "pixman-fast-path.c", line 1390: warning: result of paste undefined and not portable: 8888_8888_cover_ "pixman-fast-path.c", line 1390: warning: result of paste undefined and not portable: 8888_8888_cover_SRC "pixman-fast-path.c", line 1391: warning: result of paste undefined and not portable: 8888_8888_none_ "pixman-fast-path.c", line 1391: warning: result of paste undefined and not portable: 8888_8888_none_SRC But the result seems to be ok, however we also build on solaris 6,7 and 8 and these do cause compilation errors by inserting whitespace into the paste. Running the preprocessor on pixman-fast-path.c revels this (tidied up a little): ... static void fast_composite_scaled_nearest_8888 _8888_cover_SRC (... static void fast_composite_scaled_nearest_8888 _8888_none_SRC (... As you can see, the preprocessor has unhelpfully inserted whitespace, and the compiler doesn't like that very much: "pixman-fast-path.c", line 1390: cannot have void object: fast_composite_scaled_nearest_8888 "pixman-fast-path.c", line 1390: syntax error before or at: _8888_cover_SRC We hacked around it by duplicating the FAST_NEAREST_MAINLOOP macro in pixman-fast-path.h but removing the pasting in scale_func_name, then using the new non-pasting macro in the FAST_NEAREST macro and doing the pasting there instead. It's attached.
Hmm, that looks weird. Especially the way how 'fast_composite_scaled_nearest_8888 _8888_cover_SRC' was split. Does the "warning: result of paste undefined and not portable: 8888_8888_cover_" actually mean that each and every intermediate result of using paste preprocessor operator ('##') has to be a valid C identifier? Which means that it should never start with a digit?
(In reply to comment #1) > Does the "warning: result of paste undefined and not portable: > 8888_8888_cover_" actually mean that each and every intermediate result of > using paste preprocessor operator ('##') has to be a valid C identifier? Which > means that it should never start with a digit? I think that's what it's meant to mean, but it's inconsistent, for example: % cat a.c extern void val1(int, int); int f_1_y, f_yx, f_1stry; f_1_y = f_yx = f_1stry = 0; #define MACRO(a,b) val1(f_ ## a, f_ ## b) #define MACRO1(a,b,c) MACRO(a ## str ## b, b ## c) #define MACRO2(a,b,c) MACRO(a ## _ ## b, b ## c) MACRO1(1,y,x); MACRO2(1,y,x); % /opt/solstudio12.2/bin/cc -E a.c # 1 "a.c" extern void val1(int, int); int f_1_y, f_yx, f_1stry; f_1_y = f_yx = f_1stry = 0; # 7 val1 ( f_1stry , f_yx ); "a.c", line 8: warning: result of paste undefined and not portable: 1_ "a.c", line 8: warning: result of paste undefined and not portable: 1_y val1 ( f_1_y , f_yx ); #ident "acomp: Sun C 5.11 SunOS_i386 145355-01 2010/10/11" So, it only complains on the paste with the underscore in it, but not when there is some other string, also on earlier solaris this gives: % /opt/SUNWspro/bin/cc -E a.c # 1 "a.c" extern void val1(int, int); int f_1_y, f_yx, f_1stry; f_1_y = f_yx = f_1stry = 0; # 7 val1 ( f_1stry , f_yx ); val1 ( f_1 _y , f_yx ); #ident "acomp: Sun C 5.8 Patch 121015-07 2009/04/22" Whitespace only inserted in the case with the underscore too. Ick.
It seems to be that this actually is a buggy preprocessor. What is seems to be complaining about is this from 6.10.3.3 of http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf : If the result [of the paste operator] is not a valid preprocessing token, the behavior is undefined. However, it also says in 6.4.8: A preprocessing number begins with a digit optionally preceded by a period (.) and may be followed by valid identifier characters and the character sequences e+, e-, E+, E-, p+, p-, P+, or P-. which means something like 8888_8888_none_SRC is in fact a valid preprocessing token. I don't mind applying some workaround, but we will have to come up with one that doesn't involve duplicating this much code.
Created attachment 41605 [details] [review] a bit less intrusive workaround This is a modification of the patch which introduces less changes. Could you test it with old sun studio? I have just install solaris studio 12.2 myself, but I still wonder if just having no warnings in it would be enough to be reasonably confident that the code would work fine with older sun studio too?
(In reply to comment #4) > This is a modification of the patch which introduces less changes. Could you > test it with old sun studio? I have just install solaris studio 12.2 myself, > but I still wonder if just having no warnings in it would be enough to be > reasonably confident that the code would work fine with older sun studio too? This is indeed better, and works for us with older sun studio also. Thank you!
The fix has been pushed to pixman git repository: http://cgit.freedesktop.org/pixman/commit/?id=ab3809f4da0d833944363c5c039c3a2e6a8389c5 Thank you for reporting, testing and constructive feedback. But I guess we are not done with proper Solaris Studio support yet, pixman has to be compiled with MMX and SSE2 optimizations disabled to pass 'make check' tests right now (at least with Solaris Studio 12.2). Do older Sun Studio/Solaris Studio releases fail in a similar way?
(In reply to comment #6) > The fix has been pushed to pixman git repository: > http://cgit.freedesktop.org/pixman/commit/?id=ab3809f4da0d833944363c5c039c3a2e6a8389c5 Thanks! > But I guess we are not done with proper Solaris Studio support yet, pixman has > to be compiled with MMX and SSE2 optimizations disabled to pass 'make check' > tests right now (at least with Solaris Studio 12.2). > > Do older Sun Studio/Solaris Studio releases fail in a similar way? All our solaris systems < 10 are sparc, so we don't know. We did encounter the issue with solaris 10/x86/sun studio 12.2, but just --disable'd mmx and sse2.
OK, I see, the platform is indeed set to "Other/Solaris" for this bug. So I guess it can be considered resolved now. And new bugs can be filed about the remaining MMX/SSE2 issues.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.