views:

849

answers:

4

I encountered following weird behavior yesterday. It seems a compiler bug to me or is there a something that I've missed? I was wrapping Facebook Connect for iPhone's Objective-C classes with Objective-C to C++ adaptor classes, so that they could be used from our own OpenGL/C++ code more conveniently.

The following code reveals the problem. In the first variant below, the compiler compiles but messes up the vtables and thus wrong method is called. In the second variant, we get a compiler error which indicates that gcc is confused.

Comments try to explain the situation in more detail.

#include <iostream>
#import <Foundation/Foundation.h>

// An abstract C++ interface
class Foo_cpp {
public:
    virtual void foo() = 0;
};

// Another abstract C++ interface
class Bar_cpp {
public:
    virtual void bar() = 0;
};


// An Objective-C to C++ adaptor. 
// It takes a C++ interface Foo. When it's do_foo method is called it
// delegates call to Foo::foo.
@interface Foo_objc : NSObject {
    Foo_cpp* foo_cpp_;
}
@end

@implementation Foo_objc

- (id)init:(Foo_cpp*)foo {
    self = [super init];
    if (self) {
        foo_cpp_ = foo;
    } 
    return self;
}

- (void) do_foo {
    std::cout << "do_foo: ";
    foo_cpp_->foo();
}
@end 

// Another Objective-C to C++ adaptor. 
@interface Bar_objc : NSObject{
    Bar_cpp* bar_cpp_;
}
@end 

@implementation Bar_objc

- (id)init:(Bar_cpp*)bar {
    self = [super init];
    if (self) {
        bar_cpp_ = bar;
    }
    return self;
}

- (void) do_bar {
    std::cout << "do_bar: ";
    bar_cpp_->bar();
}
@end 

// Main class implements both abstract C++ interfaces (which will
// confuse the compiler as we shall see). 
// It constructs two Objective-C to C++ adaptors as a members and
// tries to pass itself as a C++ delegate for these adaptors.
class Main : public Foo_cpp, public Bar_cpp {
public:
    Foo_objc* foo_;
    Bar_objc* bar_;

    Main() {
        // We try to construct two objective-c to c++ adaptors Foo_objc and
        // Bar_objc. 
        // 
        // We expect output of 
        // [foo_ do_foo];
        // [bar_ do_bar];
        // to be
        //   do_foo: foo
        //   do_bar: bar
#if 0
        // This variant compiles but the compiler messes up
        // the vtables. When do_bar() is called, we expect
        // bar() to be called via Bar_objc, but instead
        // foo() is called from both adaptors.
        // Output is
        //    do_foo: foo
        //    do_bar: foo  !!!! Calls wrong method !!!!
        foo_ = [[Foo_objc alloc] init:this];
        bar_ = [[Bar_objc alloc] init:this];

        [foo_ do_foo];
        [bar_ do_bar];
#else 
        // Now, this variant tries to help the compiler by passing 
        // |this| via a variable of the correct interface type.
        // It actually reveals the confusion that the compiler
        // is having. Seems like a bug in the compiler.
        Foo_cpp* iface = this;
        foo_ = [[Foo_objc alloc] init:iface];

        Bar_cpp* iface2 = this;
        // Error we get is on the next code line.
        //   $ g++ -x objective-c++ -lobjc mheritance_test.mm
        //   mheritance_test.mm: In constructor ‘Main::Main()’:
        //   mheritance_test.mm:107: error: cannot convert ‘Bar_cpp*’ to ‘Foo_cpp*’ in argument passing
        bar_ = [[Bar_objc alloc] init:iface2];

        [foo_ do_foo];
        [bar_ do_bar];
#endif

    }

    ~Main() {
        delete foo_;
        delete bar_;
    }

    virtual void foo() {
        std::cout << "foo" << std::endl;
    }

    virtual void bar() {
        std::cout << "bar" << std::endl;
    }

};

int main() {
    Main m;
}

The problem occurs with iPhone SDKs and Mac's own g++ and with versions 4.0.1 and 4.2. Is there something I've understood incorrectly or is this a bug in g++?

UPDATE My example contained an accidental bug pointed out Tyler and Martin York, but it isn't the problem here. Below is an updated example.

#include <iostream>
#import <Foundation/Foundation.h>

// An abstract C++ interface
class Foo_cpp {
public:
    virtual void foo() = 0;
};

// Another abstract C++ interface
class Bar_cpp {
public:
    virtual void bar() = 0;
};

// An Objective-C to C++ adaptor. 
// It takes a C++ interface Foo. When it's do_foo method is called it
// delegates call to Foo::foo.
@interface Foo_objc : NSObject {
    Foo_cpp* foo_cpp_;
}
@end

@implementation Foo_objc

- (id)init:(Foo_cpp*)foo {
    self = [super init];
    if (self) {
        foo_cpp_ = foo;
    } 
    return self;
}

- (void) do_foo {
    std::cout << "do_foo: ";
    foo_cpp_->foo();
}
@end 

// Another Objective-C to C++ adaptor. 
@interface Bar_objc : NSObject{
    Bar_cpp* bar_cpp_;
}
@end 

@implementation Bar_objc

- (id)init:(Bar_cpp*)bar {
    self = [super init];
    if (self) {
        bar_cpp_ = bar;
    }
    return self;
}

- (void) do_bar {
    std::cout << "do_bar: ";
    bar_cpp_->bar();
}
@end 

class Main : public Foo_cpp, public Bar_cpp {
    void foo() { 
        std::cout << "foo" << std::endl;
    }
    void bar() {
        std::cout << "bar" << std::endl;
    }
};

int main() {
    Main* m = new Main;    
#if 0 
    // Compiles but produces
    //   do_foo: foo
    //   do_bar: foo !!! incorrect method called !!!
    Foo_objc* fo = [[Foo_objc alloc] init:m];
    Bar_objc* bo = [[Bar_objc alloc] init:m];
#else 
    // Doesn't compile
    Foo_objc* fo = [[Foo_objc alloc] init:(Foo_cpp*)m];
    Bar_objc* bo = [[Bar_objc alloc] init:(Bar_cpp*)m];
    // A line above produces following error
    //    mheritance_test2.mm: In function ‘int main()’:
    //    mheritance_test2.mm:82: error: cannot convert ‘Bar_cpp*’ to ‘Foo_cpp*’ in argument passing
#endif
    [fo do_foo];
    [bo do_bar];
}

UPDATE 2 If init: methods of Fooobjc and Barobjc are renamed to initfoo: and initbar: then it works correctly, but I still can't explain what is the problem with the code. Could it be related how Objective-C creates a method signatures?

+2  A: 

It's because you're trying to call virtual methods on an object before the constructor is done executing.

I did a test using your first method, just moving the

[foo_ do_foo];
[bar_ do_bar];

methods outside the constructor, and it worked for me.

Added This is basically item 9 from Scott Meyers' Effective C++: Never call virtual functions during construction or destruction.

The Meyster would probably have a fit about your destructors being non-virtual as well (item 7).

Usually I get annoyed when people quote Meyers to me, but in this case I hope it's mostly helpful!

Tyler
+1 for a good idea. :-)
Martin York
Thanks for pointing out the bug in the example, but unfortunately this doesn't solve the problem and it couldn't explain the compiler error for the second variant. My original code didn't do calls inside the constructor, I accidentally made a mistake when writing the example.
tequilatango
Using variant 1, I can't see a problem any more in my own version of your code (which does not make the calls in the ctor). If you still want further help on variant 1, it would be nice to see what the problem is beyond making virtual calls in the ctor.
Tyler
Tyler, sorry about a bit harsh tone in my previous comment, it was not intended and I'm grateful for your help. Can you post compiler version and flags that you are using? It happens for me with different compilers, but e.g. following reproduces the problem on Leopard. $ /Developer/usr/bin/g++-4.2 -x objective-c++ -lobjc -framework Foundation mheritance_test2.mm
tequilatango
No worries, mr. tango! I appreciate a good problem like this. Thanks for posting more details.
Tyler
+3  A: 

It is undefined behavior to call virtual methods from inside the constructor.

Since you are calling both foo() and bar() from Main() I would not expect you to have well defined output. But doing the calls after the object has been constructed should work. Try:

int main()
{
    Main m;
    [m.foo_ do_foo];
    [m.bar_ do_bar];
}
Martin York
Psyche. (same answer same time). Good problem-solving, Martin.
Tyler
What you say makes perfect sense and was the first thing that came into my head, but it doesn't explain the type confusion in assignment.
Michael Krelin - hacker
OK. I will have to look that up. But I suspect that the type information needed to convert between the child and parent types is not fully constructed until after the constructor is complete.
Martin York
Look up my "pseudo-answer", maybe that will feed you some ideas?
Michael Krelin - hacker
And no, construction has nothing to do with it because it's a compile-time error and because you can safely take construction out of the equation.
Michael Krelin - hacker
hacker is right, this has nothing to do with the bug in my example, because it can't explain the error message.
tequilatango
Martin, for what I can tell by your tags — you're a c++ programmer (like me) and we're spoiled with strict typing. After playing a bit and putting myself in objective c shoes it turns out that we're facing the downside of weak typing here (see my answer for details).
Michael Krelin - hacker
+6  A: 

I'm editing out my hints on the answering, since I completed the mission ;-)

I am not an Objective-C programmer, but driven by curiosity I couldn't help wondering what's going on and played with the code a bit. What I've found out that the problem surfaces after commenting out everything but Foo* and Bar* parts and adding the following line to the main():

Bar_objc *bo = [[Bar_objc alloc] init:(Bar_cpp*)0];

After playing a bit I figured that it must have something to do with not quite defined result of the alloc message. Which is fixed by splitting the assignment above in two:

Bar_objc *bo = [Bar_objc alloc]; [bo init:(Bar_cpp*)0];

That works just fine. So does casting the alloc results (see the code below). Alternatively, this can be fixed (I believe) with different names for initializers. Maybe also reimplementing alloc. No idea.

The full code with multiple inheritance (it has some other minor changes - I've changed class/public pairs to structs for brevity, removed calling virtuals in constructors, changed delete invocations to dealloc messages, maybe something else):

#include <iostream>
#import <Foundation/Foundation.h>

struct Foo_cpp { virtual void foo() = 0; };
struct Bar_cpp { virtual void bar() = 0; };

@interface Foo_objc : NSObject {
    Foo_cpp* foo_cpp_;
}
- (id)init:(Foo_cpp*)foo;
- (void)do_foo;
@end
@implementation Foo_objc : NSObject {
    Foo_cpp* foo_cpp_;
}
- (id)init:(Foo_cpp*)foo {
    if( self = [super init] ) foo_cpp_ = foo;
    return self;
}
- (void) do_foo { std::cout << "do_foo: "; foo_cpp_->foo(); }
@end 

@interface Bar_objc : NSObject {
    Bar_cpp* bar_cpp_;
}
- (id)init:(Bar_cpp*)bar;
- (void)do_bar;
@end 
@implementation Bar_objc : NSObject {
    Bar_cpp* bar_cpp_;
}
- (id)init:(Bar_cpp*)bar {
    if( self = [super init] ) bar_cpp_ = bar;
    return self;
}
- (void) do_bar { std::cout << "do_bar: "; bar_cpp_->bar(); }
@end 


struct Main : public Foo_cpp, public Bar_cpp {
    Foo_objc* foo_;
    Bar_objc* bar_;

    Main() {
        foo_ = [(Foo_objc*)[Foo_objc alloc] init:this];
        bar_ = [(Bar_objc*)[Bar_objc alloc] init:this];
    }

    ~Main() { [foo_ dealloc]; [bar_ dealloc]; }

    virtual void foo() { std::cout << "foo" << std::endl; }
    virtual void bar() { std::cout << "bar" << std::endl; }
};

int main() {
    Main m;
    [m.foo_ do_foo];
    [m.bar_ do_bar];
}

The result:

do_foo: foo
do_bar: bar

The bottom line: I take it due to somewhat weak typing and possibility to send messages to the objects regardless of types, it's better to have no messages with the same name, but different parameters.

Michael Krelin - hacker
Thanks for tip. Changing the order of two Objective-c classes changes the error messages. I'll try to split it between several files and see what happens.
tequilatango
I moved declarations of Bar_cpp and Bar_objc to bar.h and implementation to Bar.mm and did likewise to Foo. Still reproducable.
tequilatango
This gets interesting, if I rename init: methods of Bar_objc and Foo_objc to a different name, e.g. init_bar: and init_foo: respectively, then it works.
tequilatango
tequilatango, I've just separated files and it worked for me. I'll edit my answer to include files now.
Michael Krelin - hacker
tequilatango, no, it was just a matter of headers imports ;-)
Michael Krelin - hacker
I mean order of imports. I accidentally reverted the order and it was effectively what you've reported in your first comment.
Michael Krelin - hacker
Thanks. What happens if you add || Foo_objc *fo = [[Foo_objc alloc] init:(Foo_cpp*)0]; || in main after Bar_objc* bo? For me the problem still persists in that case.
tequilatango
Right, like I said - it was my bad, I accidentally imported in the wrong order and haven't changed the test. I find it really strange. But then again - I'm not an obj-c programmer, just being curious.
Michael Krelin - hacker
Thanks for investigating this in any case. I'm asking in Apple's dev forums, if somewhere there could explain the phenomenon.
tequilatango
Remember to report back - it's of no importance for me, but I'm curious;-)
Michael Krelin - hacker
I nailed it;-) Will update the answer now.
Michael Krelin - hacker
See the answer I got from Apple Developer forums, it explained the problem quite clearly. I gave correct answer to you though, as it seemed a right thing to do.
tequilatango
Again, thanks, but I maintain, I not only found the solution, but diagnosed the cause as well ;-)
Michael Krelin - hacker
Isn't it because the standard message [alloc] returns an uninitialized NSObject?
mos
No, mos, it is because it returns `id`-typed object. If the problem was lack of initialization you wouldn't be able to invoke `init` at all, right?
Michael Krelin - hacker
See https://twitter.com/gparker/status/4295825015 where the guy at Apple in charge of such things confirms what @hacker is saying. Also note that init: is not a very "Objective-C"ish method name; it should probably be initWithFooCpp: and initWithBarCpp or somesuch. Aside from being more standard, doing that would also eliminate the problem.
TALlama
TALIama, thanks, it's been interesting to look at your exchange. And no, it's not the reason to avoid c++ at all costs. Actually, c++, objective c and other sensible languages should not fight each other in front of phps and alikes.
Michael Krelin - hacker
Now init: was just used in this abstract example. In the original it was initWithDelegate: which I find quite Objective-Cish name. I think that trying to name every initializer with an unique name is still an error-prone strategy, because, well, to err is human. IMO a compiler should warn by default, when it's confused and not pick something silently. gparker suggested using -Wstrict-selector-match which is really what the compiler should do by default.
tequilatango
I'd say the best practice here would be having typename in the name or in any other way being more precise about it. But, like I said, I'm not an obj-c hacker, so I'm not aware of best practices here and lack knowledge to make them up ;-)
Michael Krelin - hacker
+3  A: 

A comment from Apple Developer forums explained the problem:

The problem is that you have multiple methods call -init: that take different parameter types. +alloc returns (id), so the compiler has to guess which -init: method to call. In this case, it guesses wrong and passes the wrong pointer to the multiply-inherited object.

-Wstrict-selector-match will force a compiler warning, but the compiler should have warned about the ambiguity even without that option. You should file a bug report.

One solution is to rename your methods -initWithFoo: and -initWithBar: to avoid the confusion. Another solution is to type-cast the result of +alloc before calling -init:.

I'll mark @hacker's answer as a correct though, as it was correct. Knowing about -Wstrict-selector-match is a good additional info.

tequilatango
Thank, but hey, it wasn't just close enough it is almost verbatim copy ;-) Not bad for non-objc hacker guessing it a hard way ;-)Funnily enough, I believe I've tried -Wstrict-selector-match trying to get it to hint me at the problem and it didn't.
Michael Krelin - hacker
Yeah, you are right, your explaination was similar, my bad. Kudos! And thanks a lot of spending time on this.
tequilatango
BTW, -Wstring-selector-match catched the problem for me (g++-4.2) and explained it. I think I'll keep it enabled when doing Obj-C++ (-Wall doesn't turn it on) although it seems that it can cause annoying warnings in some other cases.
tequilatango
You're welcome. I actually couldn't help resist my own curiosity, but my luck is better than cat's so far ;-)Then perhaps I've given up on trying objective-c specific warning before I got to `-Wstrict-selector-match` ;-) (that was gcc 4.3, I believe).
Michael Krelin - hacker