slice2cpp output does not compile with standards conformant compiler

[rspencer]

Although GCC may accept invalid C++ code and produce a program from it, not all compilers are so forgiving. In attempting to compile the C++ code generated by slice2cpp with the clang compiler, I am unable to because it reports undefined identifiers.

The C++ standard says that unqualified identifiers used within a template definition must be defined before the template definition, note before the template instantiation. However, GCC accepts it as long as its defined before the template instantiation. clang is more standards compliant than GCC in this area and so it rightfully gives an error. Other compilers that implement the C++ standards strictly will also generate errors on this input.

This problem occurs with the use of "upCast" and "MarshalException" in the generated template code. MarshalException is declared but not defined sufficiently for usage in the generated code. upCast is not declared until after the template definition.

The problem was posted as a bug against clang but it is unfortunately a bug in ICE. You can get all the gory details, here:

Bug 7274 – Clang gives errors that gcc doesn't when compiling output of slice2cpp

It would be good if ICE compiled with clang as its optimizers may do a better job than GCC's and I for one would use a clang compiled version of ICE over a GCC compiled version of ICE. clang's generally a faster compiler than others, too.

[mes]

Hi Reid,

Thanks for reporting this.

Regards,
Mark

[rspencer]

I now have much of ICE compiling with clang but I still get errors like this:

In file included from Acceptor.cpp:10:
In file included from ../Ice/Acceptor.h:14:
In file included from ../Ice/AcceptorF.h:15:
../../include/Ice/Handle.h:66:13: error: use of undeclared identifier 'upCast'
upCast(this->_ptr)->__incRef();

The problem is that, according to the C++ standard, upCast must be defined before the Handle template is defined. The reason it isn't is because of bugs in GCC:

Bug 16635 - g++ instantiates templates at the wrong place
Bug 23885 - incorrect template two-stage name-lookup

To deal with this, I'm going to work up a conditional compilation patch
for this which I hope ZeroC will accept so that ICE can be compiled with either GCC or clang.

[rspencer]

In the comment in cpp/include/Ice/Handle.h there is a note about a gcc bug:

// We include ProxyHandle.h here to make sure that the Ice::ProxyHandle
// template is defined before any definition of upCast().
//
// See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25495 for information
// on why this is necessary.

That bug indicates that the source code example is invalid C++ and that version 4.1.0 of gcc was fixed to reject such code. When the example code is compiled with clang, it also rejects it like this:

TestCase2.cpp:10:24: error: no matching function for call to 'incRef'
    Handle(T* r = 0) { incRef(r); }
                       ^~~~~~
TestCase2.cpp:23:24: note: in instantiation of member function 'Handle<Test::Process>::Handle' requested here
    ::Test::ProcessPrx process;
                       ^
TestCase2.cpp:3:6: note: candidate function not viable: cannot convert argument of incomplete type 'Test::Process *' to 'Dummy1 *'
void incRef(Dummy1*);
     ^
TestCase2.cpp:4:6: note: candidate function not viable: cannot convert argument of incomplete type 'Test::Process *' to 'Dummy2 *'
void incRef(Dummy2*);
     ^
1 error generated.

What this means is that a matching "incRef" overload needs to be at least declared before the template is instantiated. This error is not superfluous but quite legitimate. Although clang is a new C++ compiler, it is much more standards compliant than other offerings and it is pointing out here that the overload needs to be declared ahead of time.

This is related to the problem with upCast. The upCast methods need to be declared before templates that use them are instantiated.

[rspencer]

I attempted today to produce a patch that would fix the issue at hand.
Unfortunately, there are many, many, many more issues that need to be
fixed in ICE where it does not conform to C++ standards. Consequently,
I'm giving up on trying to produce a patch to get ICE to compile with
clang. What I will do is offer to assist the ZeroC staff should they wish
to correct their code, make it standards compliant and also make it
compile with Clang. I had hoped to compile with Clang so that I could get bitcode output and include ICE as part of my runtime. However, I may have to look for alternatives instead, which is a pity, because I really like ICE.

[bernard]

Hi Reid,

Unless a commercial customer needs support for this compiler, it's very unlikely we'll port Ice to this compiler.

Whether clang is right and the other compilers are wrong with respect to C++ standard compliance is beside the point. We support the C++ compilers our customers use, which also happen to be the most common C++ compilers out there.

Best regards,
Bernard

[rspencer]

That's fine, Bernard. I wouldn't expect anything else; it only makes business sense. I expect at some point (1-2 years) that clang will take over from GCC as the primary compiler for open source UNIX systems. There is currently a lot of interest in llvm/clang and its support for a variety of operating systems is improving. Distributions such as OpenBSD are already in the process of making the switch for the next release. The reasons are clear: clang is more standards compliant and it generates faster and often smaller code. I hope you'll take this into account in your future business plans because the lack of clang support may exclude potential customers for ICE (like me). Until then, I can get by with GCC.

[kpfleming]

Keep in mind also that the next major release of Xcode for Mac OSX is highly likely to include LLVM/clang as the primary (if not the only) toolchain for C and C++ code. When this occurs, if Ice can't be built using Xcode, anyone trying to use Ice on Mac OSX will be in trouble.

We are thinking about dealing with this already in Asterisk SCF because we are also considering the usage of a number of C++0x features that are already implemented in GCC and Visual Studio 2010... it is likely that LLVM/clang will also support our desired feature set before we release our first version, and thus this is our route to having support for Mac OSX (because the current Mac OSX GCC is woefully out-of-date and has no support for C++0x features at all).

To some extent I'm willing to put some time in over the next 4-5 months to help get Ice to the point where it will build with LLVM/clang, for the reasons that rspencer has noted as well as future Mac OSX support.

[Game_Ender]

I am not a paying customer but I just wanted give another ping of support for compiling ICE with clang, because it's quickly becoming the future of C++. In addition to possibly becoming the standard Mac OS X compiler clang is also used to compile the entire google code base and check for errors.

Also having looked at the slice files a little bit I think the fix maybe be pretty simple. You posted this Bug 25495 for GCC which contains direct sample code. Here is a version of that code which compiles without errors on clang 2.9 and g++ 4.4 (Ubuntu linux). I believe all that is needed is that your slice generator place the upCast function declaration before it includes the Handle code:

namespace Test                                                                                               
{                                                                                                            
    class Process; // moved up from below                     
}                                                                                                            
void incRef(::Test::Process*); // moved up from below     
                                                                                                             
template<typename T>                                                                                         
class Handle                                                                                                 
{                                                                                                            
public:                                          
    Handle(T* r = 0) { incRef(r); }                                                                          
};                                                                                                           
       
// This dummy code can be before or after the Handle template above
class Dummy1; 
class Dummy2;  
void incRef(Dummy1*);     
void incRef(Dummy2*); 
                                                                                                             
                                                                                                             
namespace Test                                                                                               
{                                                                                                            
    // moved up: class Process;     
    typedef ::Handle<Process> ProcessPrx; 
}
// moved up: void incRef(::Test::Process*);
                                                                                                             
int main()                                                                                                   
{                                                                                                            
    ::Test::ProcessPrx process;                                                                              
    return 0;                                                                                                
}

[MKroehnert]

I was able to get libIce to compile on OS X with clang++ (it still fails on Freeze due to incorrect C++)

The only error was:

/usr/local/bin/clang++ -c -I.. -I../../include -DICE_API_EXPORTS   -g -Wall -D_REENTRANT Acceptor.cpp
In file included from Acceptor.cpp:10:
In file included from ../Ice/Acceptor.h:14:
In file included from ../Ice/AcceptorF.h:15:
../../include/Ice/Handle.h:106:13: error: use of undeclared identifier 'upCast'
            upCast(this->_ptr)->__decRef();
            ^

and it could be fixed by applying the following patch:

diff --git a/src/Ice/Acceptor.cpp b/src/Ice/Acceptor.cpp
index 7dac8b1..aacef0e 100644
--- a/src/Ice/Acceptor.cpp
+++ b/src/Ice/Acceptor.cpp
@@ -9,6 +9,10 @@
 
 #include <Ice/Acceptor.h>
 
+namespace IceInternal {
+    IceUtil::Shared* upCast(Acceptor*);
+}
+
 using namespace std;
 using namespace Ice;
 using namespace IceInternal;

Hope this helps,

Manfred

[romain]

Hi, I have a similar problem on gcc 4.7 (c++0x mode or not) when compiling a pet project (the phone book example from doc)

gcc.compile.c++ bin/gcc-c++0x/release/server.o
In file included from /usr/include/Ice/LocalObjectF.h:15:0,
                 from /usr/include/Ice/CommunicatorF.h:24,
                 from /usr/include/Ice/Initialize.h:13,
                 from /usr/include/Ice/Ice.h:13,
                 from server.cpp:1:
/usr/include/Ice/Handle.h: In instantiation of ‘IceInternal::Handle<T>::Handle(T*) [with T = Ice::Communicator]’:
/usr/include/Ice/OutgoingAsync.h:49:16:   required from here
/usr/include/Ice/Handle.h:66:13: error: ‘upCast’ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive]
In file included from server.cpp:2:0:
./Phone.h:86:26: note: ‘IceProxy::Ice::Object* IceInternal::upCast(IceProxy::Phone::PhoneEntryFactory*)’ declared here, later in the translation unit
In file included from /usr/include/Ice/LocalObjectF.h:15:0,
                 from /usr/include/Ice/CommunicatorF.h:24,
                 from /usr/include/Ice/Initialize.h:13,
                 from /usr/include/Ice/Ice.h:13,
                 from server.cpp:1:
/usr/include/Ice/Handle.h: In instantiation of ‘IceInternal::Handle<T>::~Handle() [with T = Ice::Communicator]’:
/usr/include/Ice/OutgoingAsync.h:49:16:   required from here
/usr/include/Ice/Handle.h:106:13: error: ‘upCast’ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive]
In file included from server.cpp:2:0:
./Phone.h:86:26: note: ‘IceProxy::Ice::Object* IceInternal::upCast(IceProxy::Phone::PhoneEntryFactory*)’ declared here, later in the translation unit
In file included from /usr/include/Ice/LocalObjectF.h:15:0,
                 from /usr/include/Ice/CommunicatorF.h:24,
                 from /usr/include/Ice/Initialize.h:13,
                 from /usr/include/Ice/Ice.h:13,
                 from server.cpp:1:
/usr/include/Ice/Handle.h: In instantiation of ‘IceInternal::Handle<T>::Handle(const IceInternal::Handle<T>&) [with T = Ice::Communicator; IceInternal::Handle<T> = IceInternal::Handle<Ice::Communicator>]’:
/usr/include/Ice/OutgoingAsync.h:49:16:   required from here
/usr/include/Ice/Handle.h:98:13: error: ‘upCast’ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive]
In file included from server.cpp:2:0:
./Phone.h:86:26: note: ‘IceProxy::Ice::Object* IceInternal::upCast(IceProxy::Phone::PhoneEntryFactory*)’ declared here, later in the translation unit

... and so on ...

I'm far from being an expert, but those errors really look the same as those I get as with clang-3.0.

Thank you in advance for your support.
Best regards,
Romain

PS: by the way, your Ice Manual is a great reference, thanks for that too!

[grembo]

Hi guys,

I spent some time on fixing Ice so it compiles and runs properly using clang 3.0 and gcc 4.7. Please see

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

for details.

Cheers,
Michael