backport fixes for clang compiler errors to the 0.48.x branch

[Niederstrasser]

The 0.48.x branch does not compile using the clang compiler. I have backported several of the fixes from the trunk so that the 0.48.x branch can also be compiled with clang. I'll attach several separate patches to deal with the various errors that had to be dealt with.

[Niederstrasser (niederstrasser)]

This patch fixes the compiler check in configure and removes flag that is a no-op.

The compiler check is from revision 12203. The removal of "--export-dynamic" comes from https://bugs.launchpad.net/inkscape/+bug/707205/comments/28

[Niederstrasser (niederstrasser)]

Backport of revision 12202 to 0.48.x. Fixes
src/2geom/basic-intersection.cpp
src/2geom/solve-bezier-parametric.cpp
src/color-profile.cpp

Backport of revision 10360 to 0.48.x. Fixes
src/ui/widget/registered-widget.h

[Niederstrasser (niederstrasser)]

Backport of revision 10399 to 0.48.x. Fixes
src/display/nr-filter-gaussian.cpp (variable length array of non-POD element type 'FIRValue'_

Backport of revision 12022 to 0.48.x. Fixes
src/2geom/basic-intersection.cpp (variable length array of non-POD element type 'Geom::Point')
src/2geom/solve-bezier-parametric.cpp (variable length array of non-POD element type 'Geom::Point')

[Niederstrasser (niederstrasser)]

This last patch fixes the error "variable length array of non-POD element type 'Glib::ustring' " in src/ui/dialog/inkscape-preferences.cpp:1243:29. However, I had to backport all of revision 10062, because I don't know how to edit the C code to do a minimal change to just that file. Therefore, I just searched for the earliest code that got around the issue. This unfortunately causes string changes.

[Niederstrasser (niederstrasser)]

All the above patches (EXCEPT the preferences patch) as one file.

[su_v (suv-lp)]

Clang error message mentioned in comment #4:

  CXX ui/dialog/inkscape-preferences.o
../../src/ui/dialog/inkscape-preferences.cpp:1243:29: error: variable length array of non-POD element type 'Glib::ustring'
        Glib::ustring labels[numIems];
                            ^
1 error generated.
make[2]: *** [ui/dialog/inkscape-preferences.o] Error 1

Source code:
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/RELEASE_0_48_BRANCH/view/head:/src/ui/dialog/inkscape-preferences.cpp#L1243>

Similar clang compiler errors had been tracked (and worked around) in bug #889172 - the one in the preferences dialog had been undetected at that time, apparently.

[su_v (suv-lp)]

Inkscape 0.48.x r9950 + proposed patches confirmed to compile without
failure on OS X 10.7.5 with older version of Apple's clang (Apple clang
version 3.1 (tags/Apple/clang-318.0.58) (based on LLVM 3.1svn),
installed with Xcode 4.3.2), dependencies from current MacPorts
(GTK+/X11 2.24.19, gtkmm 2.24.3 (patched), glib 2.36.3, glibmm 2.36.2)

--
(corrected version of comment)

[su_v (suv-lp)]

1) TODO: verify whether the patch 'inkscape-clang-r10062-to-0.48.x.patch' includes string changes or completely new strings - usually not allowed for backports
2) Attaching list of '[-Wformat-security]' warnings from r9950+all 4 patches from above, using (otdated) Apple clang 3.1
   (see also bug #1193025, fixed in r12563)

$ grep '\[-Wformat-security' 2013-09-21-mptrunk-048x-9950-142424.txt | wc -l
      17

[Niederstrasser (niederstrasser)]

I have a backported version of trunk r12563 from bug #1193025 that successfully clears all -Wformat-security warnings from 0.48.4. I can upload it later in the day when I have access to that machine now that LP is working.

[su_v (suv-lp)]

On 2013-09-21 14:28 +0200, ~suv wrote:
> 1) TODO: verify whether the patch
> 'inkscape-clang-r10062-to-0.48.x.patch' includes string changes or
> completely new strings - usually not allowed for backports

AFAICT the only really new string in 'inkscape-clang-r10062-to-0.48.x.patch' is:
- "Select a bitmap editor"
  (src/ui/widget/preferences-widget.cpp, line 731)

The other strings already exist in 'po/inkscape.pot', and get reused.

AFAICT there are two options:
a) add an exception to the rules (no new features and no string changes in bug-fix releases)
b) wait until someone provides a solution how to rewrite the offending piece of code (see comment #4, #6)

[su_v (suv-lp)]

Patch to fix format-security warnings (lp:inkscape/0.48.x r9950)

[Niederstrasser (niederstrasser)]

suv's patch in comment #12 is identical to what I mentioned in comment #10, so I won't bother uploading my copy.

[su_v (suv-lp)]

1) Committed:
Patches from comment #2 and #3 committed to lp:inkscape/0.48.x in revision 9951
Backport for format-security fix committed to lp:inkscape/0.48.x in revision 9952

2) Fix configure.ac:
@Alex - could you help out again and review the patch from comment #1? AFAICT it removes a variable ($cc_vers_major) which is used later (on line 730 )
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/RELEASE_0_48_BRANCH/view/head:/configure.ac#L730>
Configure doesn't fail, but there might be a better solution?

3) Remaining issue:
Fix "error: variable length array of non-POD element type 'Glib::ustring'" in ' src/ui/dialog/inkscape-preferences.cpp'
See comment #11

[Alex Valavanis (valavanisalex)]

>> @Alex - could you help out again and review the patch from comment #1? AFAICT it removes a variable ($cc_vers_major) which is used later (on line 730 )

Cherry-picked backport from lp:inkscape (r12203 and r12213).

Should be fixed in lp:inkscape/0.48.x r9954

[Alex Valavanis (valavanisalex)]

>> Fix "error: variable length array of non-POD element type 'Glib::ustring'" in ' src/ui/dialog/inkscape-preferences.cpp'

Fixed dynamic allocation in lp:inkscape/0.48.x r9955 (no string changes needed)

Build now succeeds with clang. :)

[su_v (suv-lp)]

> Build now succeeds with clang. :)

No, not on PLATFORM_OSX:

  CXXLD inkscape
  CXXLD inkview
clang: error: unsupported option '--export-dynamic'
make[2]: *** [inkscape] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2

@Alex - we need to get rid of that '--export-dynamic' linker flag AFAICT (was part of Niederstrasser's patch from comment #1)
<https://bugs.launchpad.net/inkscape/+bug/707205/+attachment/2706002/+files/048x-gcc46-configure-linker-fix.diff>

See comments in bug #707205 for details - it seems wrong to use the flag based on platform (osx) instead of compiler version/GNU linker, and the flag was removed completely in current trunk.

[Alex Valavanis (valavanisalex)]

Oops... Committed in lp:inkscape/0.48.x r9956!

[su_v (suv-lp)]

r9965 built successfully with llvm-gcc-4.2 and Apple clang 3.1 on OS X 10.7.5.

Proposing one more patch (this one for autogen.sh):
It allows to configure a checkout of <lp:inkscape/0.48.x> on systems with older Xcode versions which include automake 1.10 ('/usr/bin/automake'), as originally described in bug #992047.

AFAICT it should not cause harm on linux distros.

[su_v (suv-lp)]

Correction (typo in rev no, sorry):
- r9965 built successfully with llvm-gcc-4.2 and Apple clang 3.1 on OS X 10.7.5.
+ r9956 built successfully with llvm-gcc-4.2 and Apple clang 3.1 on OS X 10.7.5.

[Alex Valavanis (valavanisalex)]

Looks fine to me. Committed in lp:inkscape/0.48.x r9960 and tested with Ubuntu 13.04 (automake 1.11.6)

[su_v (suv-lp)]

@Alex - many thanks for your help!

@Niederstrasser - anything we missed missing? Can you confirm that the branch now builds with recent versions of clang (Apple LLVM version 4.2, Apple LLVM version 5.0)?

[Alex Valavanis (valavanisalex)]

There are still an uncomfortably large number of warnings when building with clang in the 0.48.x branch, but it would take a fairly large patch to fix them. It kind of goes against the spirit of backporting, I guess!

[Niederstrasser (niederstrasser)]

From the splash screen of a fresh unpatched bzr pull using Apple LLVM version 4.2 (clang-425.0.28) (based on LLVM 3.2svn) on 10.7/Xcode 4.6.3:

Inkscape 0.48.4 r9963

Excellent!

[su_v (suv-lp)]

On 2013-09-22 21:49 +0200, Niederstrasser wrote:
> From the splash screen of a fresh unpatched bzr pull using Apple LLVM
> version 4.2 (clang-425.0.28) (based on LLVM 3.2svn) on 10.7/Xcode 4.6.3:
>
> Inkscape 0.48.4 r9963
>
> Excellent!

Thank you for your contributions which made this possible, this help is much appreciated!