[Pixman] [PATCH] Fix pixman build with older GCC releases

Siarhei Siamashka siarhei.siamashka at gmail.com
Sat Sep 28 11:54:10 PDT 2013


On Sat, 28 Sep 2013 00:01:27 -0400
Brad Smith <brad at comstyle.com> wrote:

> On 16/09/13 4:25 PM, Søren Sandmann wrote:
> > Brad Smith <brad at comstyle.com> writes:
> >
> >> Discussion seems to have died down. Could this please be commited?
> >
> > I didn't see any reply to Siarhei's suggestion of doing it as a
> > configure check. Also, you'll need to send a patch formatted with "git
> > format-patch", with a useful conmmit log.
> 
> I looked around at the few compilers that implement this GCC-ism other 
> than GCC and that's only LLVM and PCC

For example, you forgot about the Intel compiler. Not that it is
causing any problems here, but this just shows that your knowledge
about the use of the existing compilers all over the world is a
little bit incomplete.

And guess what? The other people (me included) also don't have a
complete understanding about all the possible system configurations
where pixman is used. That's why you had been politely asked to
provide a bit more details about why exactly you are interested
in GCC 3.3 support. It's simple curiosity because your original
commit message did not try to justify the practical usefulness
of the patch in a way that is clear enough for everyone. The reply
http://lists.freedesktop.org/archives/pixman/2013-August/002842.html
helped really a lot, and IMHO this should be a part of the commit
message.

> and they were already covered by the code as it is at the moment.

That's good. And such extra details (the exact GCC version these
compilers pretend to emulate) would be also a nice addition to
the commit message.

> I don't see any reason to further complicate the diff.

Are you interested in fixing the problem? Or in having your
patch applied exactly in the way you see reasonable?

I would like to get this resolved, just because I'm the one who
introduced this regression (by relying on the __GNUC__ ifdef and
incorrectly assuming that __builtin_clz has been always supported
by GCC). Sorry for this.

But your solution does not sit well with me because it adds
even more of the fragile ifdeffery magic. What if somebody comes
tomorrow, complaining about yet another problem with this ill-fated
ifdef and suggesting to also add some check, based on something like
"defined(__FOOBAR_COMPILER__)" to it?

-- 
Best regards,
Siarhei Siamashka


More information about the Pixman mailing list