Fun with member-function pointers

D

Default User

Andrey Tarasevich said:
No. That poster is flat out incorrect.

Very interesting. Below will be a more complete example of what I'm
proposing to do in the real code. That is, the managing of the handlers,
storing the pointers, retrieving the correct one based on op code, and
calling of the function pointer, take place in the base class. All the
derived class has to do is set up the relationships between op codes and its
member function pointers to implement the operation set for that particular
module.

If this is still legal, then I'd be on pretty firm ground with my plan for
the actual code.


Brian

#include <iostream>
#include <map>
#define CALL_HANDLER(object,ptrToMember)((object).*(ptrToMember))

class tbase
{
public:
typedef int (tbase::*HandlerPtr)(int);
void write(int data, int code)
{
std::map<int, HandlerPtr>::iterator it;
it = handlers.find(code);
if (it != handlers.end())
{
// Found, call the function pointer
CALL_HANDLER(*this, it->second)(data);
}
else // unhandled code
{
// error handling
}
}
void list()
{
std::map<int, HandlerPtr>::iterator it = handlers.begin();

std::cout << "Supported Op codes:\n";
while (it != handlers.end())
{
std::cout << it->first << "\n";
it++;
}
}
protected:
tbase() : baseDataStore(0)
{
}
virtual ~tbase()
{
}
std::map<int, HandlerPtr> handlers;
int baseDataStore;
};

class test : public tbase
{
public:
test() : testDataStore(0)
{
handlers[0] = static_cast<HandlerPtr>(&test::H0);
handlers[4] = static_cast<HandlerPtr>(&test::H4);
}
virtual ~test()
{
}
virtual int H0(int data)
{
baseDataStore = data;
std::cout << "test::H0: " << baseDataStore << "\n";
return data;
}
int H4(int data)
{
testDataStore = data;
std::cout << "test::H4: " << testDataStore << "\n";
return data;
}
private:
int testDataStore;
};

int main()
{
test t;
t.list();
t.write(123, 0);
t.write(456, 4);
return 0;
}
 
A

Andrey Tarasevich

Default said:
Very interesting. Below will be a more complete example of what I'm
proposing to do in the real code. That is, the managing of the handlers,
storing the pointers, retrieving the correct one based on op code, and
calling of the function pointer, take place in the base class. All the
derived class has to do is set up the relationships between op codes and its
member function pointers to implement the operation set for that particular
module.

If this is still legal, then I'd be on pretty firm ground with my plan for
the actual code.

The code you have in your example is perfectly legal, with perfectly
defined behavior.

Again, beware of broken compilers that might not handle it correctly.
But for your specific example I don't expect any surprises from the
compiler.

Keep in mind though, that one restriction imposed on such use of
`static_cast` is that the base class should be unambiguous and
non-virtual in the derived class. (I was wrong to say earlier that
virtual inheritance in not a problem in this case).

For example, this code will not compile because the base class is ambiguous

struct B { };
struct D1 : B {};
struct D2 : B {};
struct X : D1, D2 { void foo(); };

int main() {
void (B::*p)() = static_cast<void (B::*)()>(&X::foo); // ERROR
}

And this code will not compile because the base class is virtual

struct B { };
struct D : virtual B { void foo(); };

int main() {
void (B::*p)() = static_cast<void (B::*)()>(&D::foo); // ERROR
}

As long as your base class is not involved in such inheritance contexts,
your code is fine. Note, again, that the above situations will fail to
compile (as opposed to compiling and producing undefined behavior later).

Introduction of VMT pointer late in the hierarchy (as in Marcel's
example) as well as multiple inheritance (aside from ambiguous
situations illustrated above) do not really break this functionality.
The compilers are required to make it work correctly.
 
D

Default User

Andrey Tarasevich said:
The code you have in your example is perfectly legal, with perfectly
defined behavior.

Ok. That would certainly make the code simpler and cut development time for
future modules (although of the copy-and-paste variety). Mainly it would aid
maintenance if any change to the map and call system were needed. Changing
in one place rather than ten or more would be a good thing.
Again, beware of broken compilers that might not handle it correctly. But
for your specific example I don't expect any surprises from the compiler.

VS 2010 builds the example without complaint. Results are what I would want.
Keep in mind though, that one restriction imposed on such use of
`static_cast` is that the base class should be unambiguous and
non-virtual in the derived class. (I was wrong to say earlier that virtual
inheritance in not a problem in this case).

I don't intend to use either of the inheritance types that you mentioned.

Thank you very much for your analysis and comments.



Brian
 
A

Andrey Tarasevich

VS 2010 builds the example without complaint. Results are what I would want.

Well, VS is actually the problematic one. It has a bunch of settings
(see '/vms', '/vmm' and '/vmv') intended to control the trade-off
between compiler compliance and member-function-pointer size/simplicity
in this specific kind of situation (depending on the inheritance model).

However, somehow they managed to let the Marcel's example slip through.
When the VMT pointer is introduced into the class layout late in the
hierarchy (i.e. the base class is not polymorphic, while the derived
class is), the compiler fails to produce correct code. It is especially
strange in light of the fact that all the infrastructure that is needed
to handle this situation correctly is already built into the compiler
(layout change is similar to that caused by ordinary multiple
inheritance). Yet that managed to screw this specific case up. It is
broken in VS 2005. It remains broken is VS 2010.

Of course, one can work around this issue fairly easily by moving the
VMT pointer all the way up the hierarchy, i.e. by forcefully making the
base class polymorphic (not even mentioning that it will already be the
case in [almost] all practical designs). Nevertheless, it is important
to be aware of it.
 
M

Marcel Müller

No. Use a compliant C++ compiler, and it will not fail, regardless of
the type of inheritance and/or polymorphism :)

GCC, for one example, will handle all these cases correctly.

MSVC++, incidentally, has compiler settings that make it handle multiple
(and virtual) inheritance correctly, but for some bizarre reason it
fails on your the original (simple) example regardless of the settings.

You are absolutely right. MSVC seems to be the only one that fails on
the test.

I tested gcc 3.3.5, IBM VisualAge C++, Watcom C++ and even Borland C++
from '94 passed the tests.

Seems that Microsoft has still some homework left after 15 years C++.


But the cast is risky. No more than any ordinary downcast, in principle,
Only this time the downcast applies to the this pointer. But I find
contravariance always a bit non intuitive.


Marcel
 
P

Paul

You are absolutely right. MSVC seems to be the only one that fails on
the test.

I tested gcc 3.3.5, IBM VisualAge C++, Watcom C++ and even Borland C++
from '94 passed the tests.

Seems that Microsoft has still some homework left after 15 years C++.

That seems like a pretty big statement. Where exactly are microsoft
compilers non compliant?

Or is it a case that *you* non compliant and the Microsfot compiler is
fine?

Please provide some concrete facts that support your claim and it will
be investigated.
 
A

Andrey Tarasevich

Please provide some concrete facts that support your claim and it will
be investigated.

Marcel has already provided a compact example that causes Microsoft
compiler to exhibit incorrect behavior. I'll reproduce an equivalent
example here in more compact form

#include <iostream>

struct tbase {
int x;
tbase() : x(42) {}
};

struct test : tbase {
virtual ~test() {}
void f() { std::cout << "x=" << x << "\n"; }
};

int main() {
void (tbase::*p)() = static_cast<void (tbase::*)()>(&test::f);

test t;
(t.*p)();
}

This code works incorrectly in MSVC++ 2005 and MSVC++ 2010. Function
`test::f` called indirectly by `(t.*p)()` call receives incorrect value
of `this` pointer, which leads to incorrect value of `x` being printed.
This happens regardless of the compiler settings.

The compiler understands in advance that the code will not work. It
issues a warning

warning C4407: cast between different pointer to member representations,
compiler may generate incorrect code

in response to the above `static_cast`. This is, of course, not an
excuse, since the language standard requires this code to work correctly.

MSVC++ compilers have quite a few blatant standard violations. I collect
them in my "notebook" here

http://atarasevich.blogspot.com/2008/02/microsoft-vs2005-c-non-compliance.html
http://atarasevich.blogspot.com/2008/02/microsoft-vs2005-c-non-compliance_07.html
http://atarasevich.blogspot.com/2008/02/microsoft-vs2005-c-non-compliance_08.html

What I say there applies to the 2005 version, but quick checks shows
that virtually all these issues persist in 2010 version.

This problem is not included yet. I'll include it shortly.
 
P

Paul

Marcel has already provided a compact example that causes Microsoft
compiler to exhibit incorrect behavior. I'll reproduce an equivalent
example here in more compact form

   #include <iostream>

   struct tbase {
     int x;
     tbase() : x(42) {}
   };

   struct test : tbase {
     virtual ~test() {}
     void f() { std::cout << "x=" << x << "\n"; }
   };

   int main() {
     void (tbase::*p)() = static_cast<void (tbase::*)()>(&test::f);

     test t;
     (t.*p)();
   }

This code works incorrectly in MSVC++ 2005 and MSVC++ 2010. Function
`test::f` called indirectly by `(t.*p)()` call receives incorrect value
of `this` pointer, which leads to incorrect value of `x` being printed.
This happens regardless of the compiler settings.

The compiler understands in advance that the code will not work. It
issues a warning

warning C4407: cast between different pointer to member representations,
compiler may generate incorrect code

in response to the above `static_cast`. This is, of course, not an
excuse, since the language standard requires this code to work correctly.
I think you are doing something wrong. Let me explain...
The pointer either points to &test::f or &tbase::f.
In this example there isn't even such a type in the tbase class. I
mean:
void tbase::somefunctionname() //type does not exist.
Even if such a type did exist I still don't think that this type is
the same as :
void test::somefunctionname();

I would like to see what you read in the ISO standard that supports
this behaviour because it just does't seem right.


MSVC++ compilers have quite a few blatant standard violations. I collect
them in my "notebook" here

http://atarasevich.blogspot.com/200...pot.com/2008/02/microsoft-vs2005-c-non-compli...

What I say there applies to the 2005 version, but quick checks shows
that virtually all these issues persist in 2010 version.

This problem is not included yet. I'll include it shortly.
Thanks for this list.
I would hold back with adding this latest non-compliance because I'm
not so sure it actually is a non-compliance. Perhaps you should start
a thread in the std usernet group and get the standards people
involved in a discussion about it.
 
A

Andrey Tarasevich

Paul said:
I think you are doing something wrong.

No, I don't.
Let me explain...
The pointer either points to &test::f or &tbase::f.
In this example there isn't even such a type in the tbase class.
I mean:
void tbase::somefunctionname() //type does not exist.

It is not a type. It is a member. And the language specification does
not require such member to exist in that class.
Even if such a type did exist I still don't think that this type is
the same as :
void test::somefunctionname();

No, it is not the same. Which is why a `static_cast` is required.

The "why it shouldn't work" explanation you are trying to provide has
already been provided. While intuitively you reasoning is
understandable, it is incorrect. The language specification requires
this code to work as I expect it to work. The relevant language can be
found in the sections on `static_cast` (5.2.9/9) and on operators
`->*`/`.*` (5.5)
I would like to see what you read in the ISO standard that supports
this behaviour because it just does't seem right.

5.2.9/9 and 5.5.
...
I would hold back with adding this latest non-compliance because I'm
not so sure it actually is a non-compliance. Perhaps you should start
a thread in the std usernet group and get the standards people
involved in a discussion about it.

While I'm not a member of the standards committee, I'm sufficiently
proficient with the language standard to make a conclusive verdict on
the validity of the code in this case. Moreover, this specific issue is
not exactly rocket science. It is described in the standard in a rather
straightforward language. In other words, consider myself "standards
people" for the purposes of this discussion :)

Again, the issue we are discussing here is very basic and has been
discussed many times before (in "std usernet group" as well). This
functionality exists in C++ since C++98 after all. The answer is
well-known. And the code I provided is required to work as I expect it
to work. I don't see the point in "starting a thread" about it. Maybe
you should "start a thread" about it, since apparently this
functionality is news for you :)
 
A

Andrey Tarasevich

Leigh said:
Presumably Microsoft chose to have a less general *default* pointer to
member representation for performance reasons.

Well, I assumed that `/vmv` compiler switch should have the same effect
as the above pragma. It is quite possible I was mistaken. Thanks, I'll
try it with the pragma.
 
A

Andrey Tarasevich

Andrey said:
Well, I assumed that `/vmv` compiler switch should have the same effect
as the above pragma. It is quite possible I was mistaken. Thanks, I'll
try it with the pragma.

Yes, Leigh. You are right. Somehow I missed the existence of `/vmg`
switch. So, I was right _originally_ by saying that it is normally
possible to make compilers behave properly using their settings :) In
case of MSVC++ it is

#pragma pointers_to_members( full_generality, ...

or, equivalently, `/vmg` switch. (Plus, possibly, inheritance model
switches.)
 
A

Andrey Tarasevich

Paul said:
I would hold back with adding this latest non-compliance because I'm
not so sure it actually is a non-compliance.

While there's no question that the MSVC++ behavior under _default_
compiler settings is non-compliant, I'm glad to tell you that MSVC++
does actually have a setting that fixes the problem.

I kinda knew that it should be fixable in MSVC++, but I couldn't find
the switch, which is why I prematurely concluded it is not fixable. The
help entry for warning C4407 only mentions `/vms`, `/vmm` and `/vmv` for
some reason. And these alone are not enough.

The primary compiler switch for these situations MSVC++ is `/vmg`.
Thanks to Leigh Johnston for reminding me about it.
 
J

Jeff Flinn

Default said:
I've been working on a project that implements instrument drivers. The API
uses operation codes to direct behavior. All the drivers are subclasses of a
common ABC. In my implementation, I used a method of mapping the op codes
to handlers for that operation. That was accomplished with pointers to
member functions.

Someone asked me why I didn't have the handler storage and lookup in the
base class. My initial response was that the pointers had different types.
Then I got thinking that such a condition doesn't normally slow us down. So
I created a test case (skipping the map and all).

This worked, in that it built and seem to run ok. I'm not overly thrilled
with the cast necessary to store the func pointer. Does anyone see a
problem? Improvements?

How about the following (untested) code that uses type erasure via
boost(or std) function and bind. This negates the need to have virtual
functions, but works fine even if they are virtual.

#include <boost/bind.hpp>
#include <boost/function.hpp>

#include <map>

struct Driver1
{
int f1(int i) { std::cout << "d1.f1: " << i << "\n"; return i; }
int f2(int i) { std::cout << "d1.f2: " << i << "\n"; return i; }
};

struct Driver2
{
int f1(int i) { std::cout << "d2.f1: " << i << "\n"; return i; }
int f2(int i) { std::cout << "d2.f2: " << i << "\n"; return i; }
};

int main()
{
typedef int OpCode;
typedef boost::function<int (int)> Fnc;
typedef std::map<OpCode, Fnc> FncMap;

Driver1 d1;
Driver2 d2;

FncMap fncMap;

fncMap[123] = boost::bind(&Driver1::f1, &d1, _1);
fncMap[456] = boost::bind(&Driver2::f1, &d1, _1);
fncMap[789] = boost::bind(&Driver2::f2, &d1, _1);

assert(1 == fncMap[123](1));
assert(2 == fncMap[456](2));
assert(3 == fncMap[789](3));

return 0;
}


Jeff
 
P

Paul

While there's no question that the MSVC++ behavior under _default_
compiler settings is non-compliant, I'm glad to tell you that MSVC++
does actually have a setting that fixes the problem.

It seems that you are right enough about the compliant behavior
though. Thanks for that.
I kinda knew that it should be fixable in MSVC++, but I couldn't find
the switch, which is why I prematurely concluded it is not fixable. The
help entry for warning C4407 only mentions `/vms`, `/vmm` and `/vmv` for
some reason. And these alone are not enough.

The primary compiler switch for these situations MSVC++ is `/vmg`.
Thanks to Leigh Johnston for reminding me about it.
Yes just as well he did *remind you* you would've added it to your
list ;)
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,482
Members
44,901
Latest member
Noble71S45

Latest Threads

Top