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

[grembo]

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 Two small patches for Freeze and IceGrid (the former potentially bad) and Improvements to unit tests (fix in encoding and FreeBSD support). 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

[joshmoore]

Hi Michael,

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

Cheers,
~Josh

[joshmoore]

A heads up: on 10.7, the build (via homebrew: https://github.com/joshmoore/homebre...d113275bd10205) 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

[grembo]

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.

[grembo]

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 Mishap in slice compilers). 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.cg...el/mcpp/files/ ) or in MacPorts. They're also hidden somewhere on the zeroc.com website.

[joshmoore]

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?

[grembo]

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 Mishap in slice compilers 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/Th...s-3.4.2.tar.gz

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

Cheers,
Michael

[grembo]

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.

[grembo]

Please find attached a patch specific to compile Ice using clang with MacPorts. This patch also contains the Patch to prevent IceGrid node and registry replica name spoofing.

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

[joshmoore]

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

[grembo]

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.

[denn]

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
@@ -13,6 +13,7 @@
#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]

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.

[blair]

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

[grembo]

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