10:18 a.m.
Apparently no-one actually tried to apply the patch you sent -- sorry
about that. (Perhaps people delayed looking at it until a revised patch
was available, according to your comment "No doubt it won't apply cleanly
against the CVS head but I'll look in to that in a few days.") I
suggest submitting to the patch tracker in future, which makes it harder
for the patch to be forgotten. (See the `Patches' link in the left
sidebar of the home page http://www.inkscape.org/.)
Given that the patch was made against 0.38.1, I'm slowly going through
the patch trying to apply the "semantic intent" of the patch, e.g.
searching for `namespace' or `[^:]rot90'.
A lot of function prototypes had different type signatures to their
implementations.
If you mean that the SunPro compiler complains about
void f(int);
...
void f(int const i) { ... }
then please file a bug against the SunPro compiler if there isn't
already a bug report for this. cv qualifiers (const, volatile) of
the actual parameters[*1] aren't part of the function signature.
[*1] "actual parameters" as distinct from things pointed to by the
parameter, e.g. void f(int const *) has a different signature from
void f(int *), but void f(int * const) has the same signature as
void f(int *).
Namespace declarations were terminated with semicolons when they
shouldn't be.
Now fixed in CVS.
class Foo {
...
+ class Bar;
+ friend class Bar;
class Bar {
Can anyone comment on this, i.e. in standard C++, are Foo::Bar private
things automatically visible to Foo methods?
A few source files used GNU extensions.
I've removed two functions marked __attribute__(__deprecated__).
Arrays whose length isn't known until runtime are a bit of a problem.
They are allowed by C99, but C++ was based on the earlier C89, and does
not (yet?) allow them.
alloca has its own portability issues (man alloca).
There exists a g_alloca macro. I don't know what it does on systems
that don't have alloca; it's #defined as alloca on my system
(GNU/Linux).
An AC_FUNC_ALLOCA autoconf macro exists. My reading is that in general
one must explicitly call alloca(0) to free allocations, which removes
much of the convenience benefit of alloca while retaining some minor
problems of alloca like uncertain deallocation point. (There'd probably
still be a performance advantage in using alloca, perhaps even on
machines where it's a function call.)
In a couple of places I've switched from alloca or runtime-length arrays
to heap allocation (g_malloc or the like). I'll probably leave the rest
until last.
Some numerical constants had ambiguous types.
I'm considering using C99 functions log2, exp2 for some of these, but am
discouraged by the portability issue (same for isnan, isfinite). I'll
address this in the next few days.
The Solaris maths library doesn't have separate functions for
single
precision values
lib.c.math in the C++ standard says that <cmath> should provide
float ceil(float) as well as double ceil(double) (and similarly floor,
fabs). I'll change from using C99-style fabsf etc. to C++-style abs
etc.
#defining isfinite as finite is wrong: they differ in their behaviour
for NaN. (isfinite(NaN) is false, finite(NaN) is true.)
isfinite is C99; finite is BSD. Unfortunately, neither is in C++ (along
with a number of other useful things like isnan). Presumably C99
additions to math.h will be incorporated into a future C++ standard. In
the meantime, they are available from g++ 3.x if _GLIBCPP_USE_C99 is
#defined as non-zero before #including any standard headers. Does
SunPro have a similar means of accessing C99 functions/macros from C++
programs?
and fpresetsticky is called fpsetsticky.
Current inkscape source has no occurrences of fpresetsticky, and one
occurrence of fpsetsticky, in main.cpp immediately before fpsetmask and
exit.
Actually, according to the freebsd man page, they aren't synonyms, but
almost opposites. Looks like someone else found a compilation problem
with fpresetsticky and made the same wrong guess that it's equivalent to
fpsetsticky.
I've now simply removed that fpsetsticky;fpsetmask immediately before
exit. Still, I'd like to know why it was put there to start with.
As for the fpsetmask(fpgetmask() & ~(divide-by-zero or
invalid-operation)) at the beginning of main(), I'm inclined to change
to simply fpsetmask(0): simpler and at least as likely to give the same
results across all platforms. We certainly don't want SIGFPE by default
for inexact operations, and giving SIGFPE for underflow/overflow is
questionable.[*2] Or perhaps explicitly mask off inexact and
under/overflow too.
[*2] Conclusion after a minute or two of thought is that we don't want
SIGFPE for under/overflow given that no code currently uses SIGFPE for
numerical correction.
I also replaced <stdint.h> with <inttypes.h>.
Apparently this has already been applied.
The only changes of dubious merit I can think of were; changing round
to
rint as I'm not sure if this will affect the semantics of the code in
questions,
Most code won't care what happens for ##.5. Perhaps we should have an
ink_round function for callers that don't care, and more verbosely-named
functions for any callers that do care.
and importing an implementation of hash_map when you should
really switch to GHashTable as the comments suggest. I couldn't see an
obvious place to initialize the GHashTables.
I haven't looked at this change yet.
Note that the floating point optimization level (-fsimple=n) must be
1
or lower as aggressive optimization will break assertions in the code.
Is there anything we need to do in configure.ac for this, or are the
default CXXFLAGS already fine?
You might supply a short README.SunPro or README.Solaris or something
file. Just the above sentence preceded by "If you use xxx then" will
suffice, where xxx might be `the SunPro compiler (version n.nn and
possibly other versions)'.
pjrm.