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.