Archived

This forum has been archived. Please start a new discussion on GitHub.

Patch for compiling Ice with clang (and gcc4.7)

Hi,

please see the attached patches that contain various fixes that enable Ice to compile using clang 3.0 and new versions of gcc. The patches have been tested (compilation and unit tests) using:
  • clang 3.0:
  • gcc 4.7.0 20120128
  • gcc 4.2.1 (to make sure nothing broke and all changes are backwards compatible)

Content
There are three patches attached, usually you only need one of them:
  • ice_for_clang.patch.txt: This contains all changes for all compilers
  • ice_for_clang_only.patch.txt: This contains only the changes necessary to compile using clang (smaller changeset)
  • freebsd_port_devel_ice.patch.txt: Patch for FreeBSD ports (see below how to use)

All three patches also contain the patches http://www.zeroc.com/forums/patches/5645-two-small-patches-freeze-icegrid-former-potentially-bad.html and http://www.zeroc.com/forums/patches/5646-improvements-unit-tests-fix-encoding-freebsd-support.html. Note that these changes only affect Ice for C++ and other language mappings have neither been changed nor tested.

What has been changed?
The patch fixes a couple of problems with the current implementation of Ice in terms of C++ standard compliance. The biggest issue is the way the upCast functions are overloaded. The standard says:
First, the compiler does unqualified lookup in the scope where the name was written. For a template, this means the lookup is done at the point where the template is defined, not where it's instantiated. Since Multiply hasn't been declared yet at this point, unqualified lookup won't find it.

Second, if the name is called like a function, then the compiler also does argument-dependent lookup (ADL). (Sometimes unqualified lookup can suppress ADL; see [basic.lookup.argdep]p3 for more information.) In ADL, the compiler looks at the types of all the arguments to the call. When it finds a class type, it looks up the name in that class's namespace; the result is all the declarations it finds in those namespaces, plus the declarations from unqualified lookup. However, the compiler doesn't do ADL until it knows all the argument types.

Since most of the upCast overloads are provided on incomplete types and after Handle has been included, the only way to lookup the correct overload (which has to be matching, since the forward declared types naturally don't provide any information on object hierarchy) is by ADL. For ADL to work, the overload has to be provided within the types namespace. Therefore a large portion of this patch is moving upCast overloads out of IceInternal into the Ice namespace. On the one hand this litters the Ice namespace with many different overloads of upCast, on the other this seemed the easiest and most straight forward solution within breaking any internals and massive testing. Since upCast is also used within generated code slice2cpp has been changed accordingly.

The patch also fixes a cyclic inclusion problem caused by clang's stricter evaluation of template types (throwing a forward declared exception from within a template class) and other minor declaration order problems and glitches (old) gcc was fine with.

The changes implemented for gcc 4.7 are mostly cosmetic, a lot of compiler warning suppression (NDEBUG/assert).

Compilation flags
clang:Plain clang++, no further parameters (therefore C++03, system's stdc++)
gcc 4.7.1: CXX=g++47 -std=gnu++11 -I/usr/local/lib/gcc47/include/c++ -Wno-deprecated-declarations
gcc 4.2.1: g++


Caveats
This didn't compile yet using -std=c++11 since there are unresolved issues with strtoll on my platform. Ultimately I would like to see this compile using clang, C++11 and clang's libc++. Right now I can't get my hands on it in a near production environment so this has to wait.

Instructions for FreeBSD ports
Assuming you want to compile Ice using clang:

Make sure you've set
CC=clang
CXX=clang++
CPP=clang -E
(e.g. in your make.conf)

Make sure you have the latest port collection (portsnap fetch update) and patch the ice port using:
patch -p0 </path/to/freebsd_port_devel_ice.patch.txt

From there it's basically make install:
cd /usr/ports/devel/ice
make install clean

Conclusion/Disclaimer
After applying the patch, compiling Ice using clang works flawlessly and only a few intentional deprecation warnings show up. Switching to C++11 and clang's own libc++ will be the next logical step. Obviously I cannot give any guarantees for any of the code, but by all I can tell Ice behaves pretty much like it should.

Cheers,
Michael

Comments

  • Hi Michael,

    On Mac 10.6, I needed to fix one issue in CtrlCHandler.cpp: https://github.com/joshmoore/zeroc-ice/commit/3fbafeed0e274b76c16ac67c99fc70d3e8d976e5. We're also in the process of testing on 10.7 and will let you know if there are any issues.

    Cheers,
    ~Josh
  • A heads up: on 10.7, the build (via homebrew: https://github.com/joshmoore/homebrew/commit/35811fbf444721a3a9eea24fabd113275bd10205) fails with:
    /usr/local/Cellar/zeroc-ice34/3.4/bin/slice2py --prefix IceBox_ --checksum -I../../slice --ice ../../slice/IceBox/IceBox.ice
    /usr/local/Cellar/zeroc-ice34/3.4/bin/slice2py --prefix IceGrid_ --checksum -I../../slice --ice ../../slice/IceGrid/Admin.ice
    /usr/local/Cellar/zeroc-ice34/3.4/bin/slice2py --prefix IceGrid_ --checksum -I../../slice --ice ../../slice/IceGrid/Descriptor.ice
    /usr/local/Cellar/zeroc-ice34/3.4/bin/slice2py --prefix IceGrid_ --checksum -I../../slice --ice ../../slice/IceGrid/Exception.ice
    /usr/local/Cellar/zeroc-ice34/3.4/bin/slice2py --prefix IceGrid_ --checksum -I../../slice --ice ../../slice/IceGrid/FileParser.ice
    /usr/local/Cellar/zeroc-ice34/3.4/bin/slice2py --prefix IceGrid_ --checksum -I../../slice --ice ../../slice/IceGrid/Observer.ice
    /usr/local/Cellar/zeroc-ice34/3.4/bin/slice2py --prefix IceGrid_ --checksum -I../../slice --ice ../../slice/IceGrid/Locator.ice
    /usr/local/Cellar/zeroc-ice34/3.4/bin/slice2py --prefix IceGrid_ --checksum -I../../slice --ice ../../slice/IceGrid/Query.ice
    /usr/local/Cellar/zeroc-ice34/3.4/bin/slice2py --prefix IceGrid_ --checksum -I../../slice --ice ../../slice/IceGrid/Registry.ice
    make[1]: *** [IceGrid_Registry_ice.py] Segmentation fault: 11
    

    Cheers,
    ~Josh

    FYI the environment:
    ==> Environment
    HOMEBREW_VERSION: 0.8.1
    HEAD: 35811fbf444721a3a9eea24fabd113275bd10205
    HOMEBREW_PREFIX: /usr/local
    HOMEBREW_CELLAR: /usr/local/Cellar
    Hardware: quad-core 64-bit sandybridge
    OS X: 10.7.3
    Kernel Architecture: x86_64
    Xcode: 4.2
    GCC-4.0: N/A
    GCC-4.2: N/A
    LLVM: build 2336
    Clang: 3.1 build 318
    MacPorts or Fink? false
    X11 installed? true
    System Ruby: 1.8.7-249
    /usr/bin/ruby => /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/bin/ruby
    Which Perl:   /usr/bin/perl
    Which Python: /usr/bin/python
    Which Ruby:   /usr/bin/ruby => /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/bin/ruby
    ==> Build Flags
    CC: /usr/bin/clang
    CXX: /usr/bin/clang++ => /usr/bin/clang
    LD: /usr/bin/clang
    CFLAGS: -Os -w -pipe -march=native -Qunused-arguments
    CXXFLAGS: -Os -w -pipe -march=native -Qunused-arguments
    CPPFLAGS: -I/usr/local/Cellar/berkeley-db46/4.6.21/include
    LDFLAGS: -L/usr/local/Cellar/berkeley-db46/4.6.21/lib
    MAKEFLAGS: -j4
    PKG_CONFIG_PATH: /usr/local/Cellar/berkeley-db46/4.6.21/lib/pkgconfig
    
  • Environment

    Are you certain you removed all remainders of previously installed Ice components? It seems like it's crashing in running/testing(?) a python stub in IceGrid, so maybe there are some leftovers.

    It would be good if you could send me:
    • The complete build output (not just the last few lines)
    • Information on the exact version of python you're using (do you also have some other version installed?) and all installed python modules
    • Information on the version of mcpp installed (this is important and a constant source of trouble). See test below.

    Try to compile this using mcpp -C:
    #ifndef MY_MODULE_ICE
    #define MY_MODULE_ICE
    
    module MyModule
    {
      struct foo
      {
        string bar; // foobar
      };
    };
    #endif
    
    and check the output.

    I'll also try to get my hands on a 10.7 machine, but this might take a few days.
  • mcpp in brew

    Hi Josh,

    I tested homebrew myself and learned, that the version of mcpp that comes with it doesn't contain the patches necessary to use slice2py (and others) successfully (see also http://www.zeroc.com/forums/bug-reports/5309-mishap-slice-compilers.html). We managed to bring those patches into FreeBSD ports as well as MacPorts, so - without analyzing your exact setup - getting the correct patches for mcpp into brew might actually solve your build problem.

    Unfortunately the guys at ZeroC don't seem to be interested in pushing their fixes upstream to sourceforge (I talked to the author and he would have been ok with that, but he couldn't do it himself). So people keep on running into this over and over again.

    To get those patches into homebrew you'll have to modify /usr/local/Library/Formula/mcpp.rb (or install mcpp from MacPorts). You can find the necessary patches in the FreeBSD ports tree (ports/155934: Incorporate patch from the Ice project into devel/mcpp and http://www.freebsd.org/cgi/cvsweb.cgi/ports/devel/mcpp/files/ ) or in MacPorts. They're also hidden somewhere on the zeroc.com website.
  • Thanks for the research, Michael. I've submitted a pull request for mcpp to mxcl/homebrew (#10443).

    Cheers,
    ~Josh.

    P.S. When you say you've talked to the author, you mean of mcpp or the patch?
  • Hi Josh,

    I talked to Kiyoshi Matsui, the author of mcpp. His response to my request was:
    From: Kiyoshi MATSUI
    Subject: Re: Fwd: Problem using slice2py with unpatched mcpp
    Date: March 25, 2011 13:43:28 CET
    To: Michael Gmelin

    Hello,

    I'm very sorry, but I have no time for mcpp now. I think the Zeroc
    patch is correct, and has no undesirable side effect. So, if someone
    make an mcpp package applying the patch, and commit it to FreeBSD or any
    other system, I will agree with it.

    --
    Kiyoshi Matsui, the maintainer of mcpp

    See also http://www.zeroc.com/forums/bug-reports/5309-mishap-slice-compilers.html for a discussion about this topic from early 2011.

    You can also find the complete patch to mcpp done by ZeroC in their third party sources download - parts of this patch are probably already done in the homebrew formula, so there is some overlap: http://zeroc.com/download/Ice/3.4/ThirdParty-Sources-3.4.2.tar.gz

    Please let me know if correcting mcpp fixes the build for you.

    Cheers,
    Michael
  • Mac OS X Lion + clang confirmed

    I just got confirmation that this builds just fine on Mac OS 10.7 (Lion) using clang 2.1, 2.9, 3.0 and 3.1. Thanks to Andrew Edem for torturing his poor little MacBook Air for these tests.

    (Note that Josh's problems were caused by using an unpatched version of mcpp. We used MacPorts for our tests, which contains a patched version of mcpp already).

    Please find attached the patch used (this is the original patch plus a one line change specific to Mac OS X as pointed out by Josh). This works on FreeBSD as well as on MacOS X.

    Apply the patch using:

    cd /path/to/Ice-3.4.2
    patch -p0 </path/to/ice_for_clang_2012-03-05.txt


    I will post a patch specific to MacPorts at a later point.
  • Patch specific to MacPorts

    Please find attached a patch specific to compile Ice using clang with MacPorts. This patch also contains the http://www.zeroc.com/forums/patches/5663-patch-prevent-icegrid-node-registry-replica-name-spoofing.html.

    Installation example:

    sudo port selfupdate
    sudo port clean ice-cpp
    cd `port dir ice-cpp`
    sudo patch -p0 < /path/to/patch.ice-cpp.macports.txt
    sudo port install ice-cpp configure.compiler=clang
  • In the way of a status update,
    • The mcpp patch has been submitted to homebrew, with which Michael's patch works fine. See PR 10975
    • I was able to backport the clang patches from 3.4 to 3.3. See commit 83132b0, which likely needs some cleaning.
    • We've posted binaries for download as well as a formula for compiling it. See this thread on the OME forums.

    Cheers and thanks for all the help!
    ~Josh
  • Patch committed to FreeBSD ports now

    The patch has been committed to FreeBSD ports today. For details see also ports/165702: [maintainer update] [patch] devel/ice: Support clang and gcc47, security fixes, cleanup.

    To upgrade to Ice 3.4.2_2 do:

    portsnap fetch update
    portupgrade -r devel/ice


    Please note that you might have to recompile dependent software outside of the package system manually.
  • Fantastic work with confirmation !

    All,

    I have gcc4.7 build systems on Solaris 10 and 11, Flavours of Linux and the favourite, FreeBSD.

    Thus far your patch is great.

    From my build platform I have initially tested the FreeBSD platform outside of ports, and have successfully built the tree bar a one liner.

    patch is

    diff -rau Ice-3.4.2/cpp/include/Slice/Preprocessor.h Ice-3.4.2.patch/cpp/include/Slice/Preprocessor.h
    --- Ice-3.4.2/cpp/include/Slice/Preprocessor.h 2011-06-16 05:43:58.000000000 +1000
    +++ Ice-3.4.2.patch/cpp/include/Slice/Preprocessor.h 2012-04-02 18:56:38.436115544 +1000

    #include <IceUtil/Shared.h>
    #include <IceUtil/Handle.h>
    #include <vector>
    +#include <cstdio>
    #ifdef __BCPLUSPLUS__
    # include <stdio.h>
    #endif

    Otherwise business as usual.
    Would be nice if the Ice team listens to the importance of Clang++ and the input of Asterix SCF in regards to library development for the C++ community.

    Regards Denn
  • benoit
    benoit Rennes, France
    Hi,

    Yes, the next Ice release should compile fine with clang++ and gcc 4.7, we already made the changes to fix the name lookup issues mentioned by Michael. A number of the FreeBSD fixes mentioned on this thread were also already on our mainline but we will look integrating the others (the test suite changes for instance).

    Cheers,
    Benoit.
  • MacPorts patches

    Hello Michael,

    Thanks for the patch, this is great. I'm working on committing it to MacPorts now. The only complication is that the patches also need to work for older Mac OS X and g++ versions, which I'll be testing now. Hopefully they are compatible and I won't need to do any per-compiler or per-OS X version changes. Do you know if there are issues with these patches on older g++ compilers?

    I don't actively monitor the forums, so for the future, if there's a way to email me or cc me on MacPorts patches, I would be happy to commit them.

    Regards,
    Blair
  • Older gcc no problem

    Hi Blair,

    I adapted those patches for MacPorts a few months ago and opened a ticket in their bug tracker. Older gcc versions still work with this no problem. On FreeBSD it also works using gcc 4.2.1 (the last one under GPLv2, which also has been used by Apple), so it *should* work out of the box. I know that it works on SL and Lion, I can't say much about 10.5 and older. Since the patch makes the code more standards-compliant and doesn't use any fancy bleeding edge rocket science, I would suspect that it compiles and runs on the same compiler/OS combinations like it did before.

    Here's the link to the ticket on MacPorts:

    https://trac.macports.org/ticket/33478

    Cheers,
    Michael
  • I've tested with homebrew on 10.5 through 10.7 successfully.

    Cheers,
    ~Josh
  • MacPorts patches committed

    Blair committed the patches to MacPorts as well, so after doing sudo port selfupdate you should be able to build the port using clang

    Michael
  • New patch for Clang, C++11 and libc++

    I created a new patch that also covers C++11 and Clang's libc++, it's in a new thread, since the patch has a broader scope than this one.

    http://www.zeroc.com/forums/patches/5817-patch-compiling-ice-clang-c-11-libc.html

    - Michael
  • simon
    simon Netherlands
    Debian Wheezy now stable and includes g++ 4.7

    Hi all,

    Just a "me too" note. Debian Wheezy (version 7.0) release is now stable (as of May 4th 2013), which we run on all our systems (currently Squeeze=v6.0, but upgrades in progress). I came across the same compilation errors on one of my dev systems after upgrading. Are there already Ice versions available (preferably .deb) that include patches to be able to compile on the current shiny Debian stable release?

    Kind regards,
    Simon de Hartog