 |
|
 |
I now have ASSERT_OVERRIDES() macros sprinkled liberally around my code and it's definitely helped avoid some nasty bugs (and would have avoided more had I used it earlier).
This one should definitely be in every C++ programmer's toolkit.
|
|
|
|
 |
|
 |
Error message returned that the file is not a zip file, or is corrupted.
William
Fortes in fide et opere!
|
|
|
|
 |
|
 |
Oops! I've uploaded a new version -- is it okay now?
-- Bart
|
|
|
|
 |
|
 |
You don't need the WITHOUT_OVERRIDE_CHECKING since no code calls or create the struct you defined in the OVERRRIDE macro
|
|
|
|
 |
|
 |
It is needed. The struct is exported, and so is the function, and some linkers may not be so smart that they can optimize this code away. For instance, I think you need to enable "function-level linking" in VC++ to enable it to detect that the struct's code can be removed.
-- Bart
|
|
|
|
 |
|
 |
It would seem from my quick tests that, if the function you are overriding is protected, the macro won't work (because the struct you declared can't call a protected function in another class).
My solution is to surround the declaration of the function ptr and the cast with
if(0)
{
}
rather than declaring a struct. This at least avoids the problem of protected functions. This probably falls foul of your test, stated in the last thread, that it should not generate any warnings, although I can't test with your compiler. All I can say is that it passes release level 4 on the Visual C++ compiler, which is probably not saying much...
|
|
|
|
 |
|
 |
To overcome the "protected" problem, you should change the OVERRIDE macro so the new struct will too inherite from the baseclass:
struct derivedclass##_##func##_override_test_class : public baseclass
Hence, you can check protected function overloading too.
|
|
|
|
 |
|
 |
Thanks for the hint. However, this doesn't work if the base class doesn't have a default constructor. I tried to solve it using an even simpler solution: I moved the use of OVERRIDE to inside the class definition and there I declared an inline function to do the tests. However, it seems that although GCC accepts &A::foo from B if A::foo is protected, Comeau does not. I think Comeau is conceptually correct in this case, because even though B::foo has access to A::foo, it could pass the pointer to the outside and break encapsulation. Unfortunately this means that I don't have any way to perform this check... unless, of course, I insert a bogus call to the base class function. Unfortunately, that might lead to errors if the base class function is pure virtual.
Let's identify the possibilities. You can access protected members in the following ways:
1. Be a derived class and *call* the function (not an option because it might be pure virtual!).
2. Be a friend of the base class. (This should have been declared by the base class but hasn't been -- I don't want to have to modify the base class just to do this kind of check!)
3. Be a friend of the derived class.
The third one seemed the most promising option. I compiled the following code using both GCC and Comeau, and although Comeau generated a variable-not-used warning at least this compiles:
class A
{
protected:
void foo();
};
class B : public A
{
friend class C;
class C
{
public:
void bar()
{
void (A::*testptr)() = &B::foo;
}
};
};
I reworked the OVERRIDE macro so that it could be used like this, and it works great. There is only one major problem: I need to generate a unique name for class C for every overriding function, and the name of the function is not enough for that! The only solution I can think of is to add another parameter that the user would have to fill with a unique number; not a pretty solution. Another solution would be to use the preprocessor's __LINE__ macro, but unfortunately that macro doesn't exist in Comeau and has a bug in GCC's preprocessor that prevents it from being used as V##__LINE__. Anyone have any ideas I could use?
-- Bart
|
|
|
|
 |
|
 |
I think there is a more simpler solution.
You said that my solution wasn't good if the base class didn't define a default constructor, then don't either, instead, define a member function that will call the code:
Instead of
derivedclass##_##func##_override_test_class_function()
write
void derivedclass##_##func##_override_test_class_function()
|
|
|
|
 |
|
 |
I'm sorry, I don't quite get what you mean. I did try to do it with a member function, as I said in my previous posting (I wrote "I moved the use of OVERRIDE to inside the class definition and there I declared an inline function to do the tests"). And my previous posting also explains where the problems are with this approach: I wrote "However, it seems that although GCC accepts &A::foo from B if A::foo is protected, Comeau does not.", which means that the trick doesn't work on all compilers. Maybe I should write more clearly? BTW, even if it did work, your trick wouldn't work as-is, because it would lead to duplicate declarations of the same function (when using OVERRIDE multiple times because of function overloads). When I tried it out I solved that by overloading the test function with the same parameter list as the overriding function. Please work with me to find a solution that works on all compilers, by finding a solution to the unique-naming issue that I brought up in my previous post. Thank you.
-- Bart
|
|
|
|
 |
|
 |
Eureka, I found it:
#define OVERRIDE(baseclass,returntype,derivedclass,func,params) \
static void func params \
{ \
struct returntype##derivedclass##_##func##_override_test_class : public baseclass \
{\
returntype (baseclass::*fntestptr) params;\
baseclass *basetestptr;\
void derivedclass##_##func##_override_test_class_function()\
{\
fntestptr = &baseclass::func;\
basetestptr = reinterpret_cast(0);\
}\
};\
}\
returntype derivedclass::func params
|
|
|
|
 |
|
 |
OK, let's take a look at this.
First of all, the construct you use at the end (basically declaring something like void A::foo()) is not standard C++.
Second, I'd give the checking function a different name than "func", otherwise they're going to call the checking func instead of the real one.
Third, this doesn't fix the problem I was describing -- some compilers (Comeau, specifically) don't allow you to take the address of baseclass::func when it's protected, even when you're inside a derived class. I compiled the following code (slightly adapted from your code so that it's standard C++):
#define OVERRIDE(baseclass,returntype,derivedclass,func,params) \
static void func##_test_func params \
{ \
struct returntype##derivedclass##_##func##_override_test_class : public baseclass \
{\
returntype (baseclass::*fntestptr) params;\
baseclass *basetestptr;\
void derivedclass##_##func##_override_test_class_function()\
{\
fntestptr = &baseclass::func;\
basetestptr = reinterpret_cast<derivedclass*>(0);\
}\
};\
}\
returntype func params
class A
{
protected:
void foo();
};
class B : public A
{
public:
OVERRIDE(A,void,B,foo,());
};
and Comeau told me:
"ComeauTest.c", line 26: error: protected function "A::foo" is not accessible
through a "A" pointer or object
OVERRIDE(A,void,foo,());
So, there you go. Try and fix that. And get it through Comeau !
-- Bart
|
|
|
|
 |
|
 |
First of all, who cares about Comeau !!!
Ask them why can't I get a pointer to a protected function, its ilogical.
Second, don't use my OVERRIDE in the definition file (.h file) just use it as your original code has: in the declaration file (.cpp file) so no problems in "standard C++" and also no name ambiguity with the static function name.
|
|
|
|
 |
|
 |
So, OK, the thing is in the cpp file, I misinterpreted things because I moved to trying solutions in the header file in the mean time. Sorry!
Anyway, I care about Comeau, as I care about writing portable code. The compiler is closer to the next version of the standard than any other compiler I know, so I like to check my code with it for portability, so that it may still work in five years.
The fact that your version of OVERRIDE can be written like this shows even more clearly that taking the address of a protected member shouldn't be allowed. If this were allowed, I'd always be able to write a standard "protection circumvention mechanism" as follows:
class A
{
protected:
void foo(); };
void bar(A a)
{
}
void baz(A a)
{
struct ProtectionCircumventer : public A
{
static void get_foo(void (A::*a_foo_ptr)())
{
a_foo_ptr = &A::foo;
}
};
void (A::*foo)();
ProtectionCircumventer::get_foo(foo);
(a.*foo)();
}
This means that, with a bit of work, I could circumvent all the protection of A or any other class! Okay, you might say, write a derived class and call it from there, that's the same thing, right? And then I would say: no, because in that case the object has to be of your derived type in order for you to be able to call A's members. So, circumvention is not possible if you forbid taking the address of protected members, and it is possible if you allow it. Unlogical to forbid it? I don't think so.
-- Bart
|
|
|
|
 |
|
 |
Hey folks, I can see I've really started a debate here. I'm sorry that I'm not making any useful contributions, but I'm living quite happily with my if(0) and my non-fundamentalist compiler, so I guess I'll leave the discussion in your capable hands.
|
|
|
|
 |
|
 |
Your macro is defined as below:
#define OVERRIDE(baseclass,returntype,derivedclass,func,params) \
struct derivedclass##_##func##_override_test_class \
{ \
returntype (baseclass::*fntestptr) params; \
baseclass *basetestptr; \
derivedclass##_##func##_override_test_class() \
{ \
fntestptr = &baseclass::func; \
basetestptr = reinterpret_cast<derivedclass*>(0); \
} \
}; \
returntype derivedclass::func params
But don't you think the statment:
basetestptr = reinterpret_cast*ltderivedclass*>(0); \
is unnecessary? Deleting it seems to make no difference from the original effect of this OVERRIDE macro.
Furthermore, your OVERRIDE macro can be further simplied:
#define OVERRIDE(baseclass,returntype,derivedclass,func,params) \
void function_derivedclass##_##func##_override_test() \
{ \
returntype (baseclass::*fntestptr) params = &baseclass::func; \
} \
returntype derivedclass::func params
I stripped everything except a core assignment-statement in the function to do the function overriding checking. Do you think I am right?
|
|
|
|
 |
|
 |
Okay, let's take this one at a time. About your first comment, that the statement
basetestptr = reinterpret_cast<derivedclass*>(0);
is redundant: it isn't, otherwise I wouldn't have put it there. If you write
class A
{
void foo() {}
};
class B
{
void foo();
};
OVERRIDE(A,void,B,foo,())
{
}
it would compile without this statement, as the only thing the rest of the macro does is to take the address of A::foo. The statement is intented to check whether B is actually a derived class of A, so that it can make sure that B::foo is actually an override of A::foo instead of that it just has the same signature (as the other part of the checking macro does).
About the second part of your comment, and the stripped version you present: yes, this is definitely simplified, but unfortunately it doesn't serve it's purpose as well. I tried to do it this way originally, but I changed it because I found that this simplicity has a disadvantage. I'll tell you what the disadvantage is: fntestptr is not used in the function, and at the highest warning level most compilers will give you a warning on this! The last thing you want is an unfixable warning for every time you use this macro, so I experimented with various compilers to see what would give me the least amount of unnecessary warnings (zero, preferably ). It turned out that the only way that I could fool both GCC and Comeau's compiler into not emitting a warning was to define a class with the test variables as its public members, and to make the class extern. If the variables were included as locals in the function GCC and Comeau would emit a warning, and if I made the class "static" (i.e., not exported) Comeau would still emit a warning about a never-instantiated non-exported class or something.
So, although you are correct in stating that the macro can be simplified code-wise, the effects of this simplification are not desirable.
-- Bart
|
|
|
|
 |
|
 |
Now, I agree that adding basetestptr = reinterpret_cast<derivedclass*>(0); is better than not adding it, which does more checking and make your code more secure. But for the issue of "some variable not used in a function" warning from the compiler, I can add (void)fntestptr; just following the assign-statement of fntestptr, which make `gcc -Wall' quiet again. And further more, I realized later that the macro can be further simplified. #define OVERRIDE(baseclass,returntype,derivedclass,func,params) \ returntype (baseclass::*baseclass##_fntestptr) params = &baseclass::func; \ baseclass *baseclass##_basetestptr = reinterpret_cast<derivedclass*>(0); \ returntype derivedclass::func params This time, no "struct", no function, but only two global variables. However, I'm not persuading you into taking my may of the OVERRIDE macro, everyone has his preference, and the macro is just a facility of carrying out code checking during coding, isn't it?
|
|
|
|
 |
|
 |
I guess that should work, and it is pretty simple indeed. Unfortunately, an issue has come up (overriding protected member functions) that precludes this solution. Also, I found that (void)fntestptr works for VC++ but not for Comeau, and I'd like it to at least compile without warnings in all of the compilers I have available. Indeed it's just a checking facility, so I don't care how simple or complicated it is, as long as it works as advertised.
-- Bart
|
|
|
|
 |
|
 |
Here is another way to force derived classes to implement function(s) from the base class. This may not be the way to go in all situations. It requires the base class's function to not be implented. class BaseClass { public: virtual void foo(int n = 3) = 0; //This makes the function a pure virtual }; class A : public BaseClass //this class is fine, because it implements foo { public: virtual void foo(int n = 3) { std::cout << "A::foo(" << n << ")" << endl; } }; class B : public BaseClass //the compiler will stop, because B does not implement all of the base's pure virtual functions. { public: virtual void foo(); }; int main() { BaseClass* baseClass = new A; baseClass->foo(); // prints "A::foo". }
|
|
|
|
 |
|
 |
Definitely a good solution when you've got the opportunity. This only works if your policy is to *only* implement functions in leaf classes. Unfortunately such a policy is not always useful. It can make some kinds of inheritance very impractical, especially when your derived class is intended to "adapt" the behaviour of a superclass (e.g., when you derive your own class from a CWnd-like class, override a callback and call the base class's implementation as part of your overridden behaviour). But still, if you have the chance to solve things like this, please do so. Macros aren't the nicest way to solve things.
BTW, there is a minor mistake in your post, as the code you've given does in fact compile. The compiler emits an error not on B's class declaration, only when you try to instantiate B, e.g. when you write new B instead of new A. The error only occurs when you try to instantiate a class that lacks an implementation for one or more of its virtual member functions.
-- Bart
|
|
|
|
 |
|
 |
You're right. The solution is only useful in certain cases. I did forget about the compiler only complaining when a class is instanciating. Thanks for reminding me.
|
|
|
|
 |
|
 |
What about using explicit overriding of abstract functions ?
You then have both advantages:
- derivation from a class which implements functions
- compile time errors without macros
I modified the sample slightly.
class BaseClass
{
public:
virtual void foo(double nu) = 0;
};
class A : public BaseClass
{
public:
virtual void BaseClass::foo(int n) // Explicit override
{
std::cout << "A::foo(" << n << ")" << endl;
}
};
class B : public A
{
public:
virtual void BaseClass::foo(int n)
{
std::cout << "B::foo" << endl;
}
};
|
|
|
|
 |
|
 |
I would like this if it worked.
I tried to compile the following code sample using GCC:
class BaseClass
{
public:
virtual void foo(int nu) = 0;
};
class A : public BaseClass
{
public:
virtual void BaseClass::foo(int n) {
std::cout << "A::foo(" << n << ")" << endl;
}
};
class B : public A
{
public:
virtual void BaseClass::foo(int n)
{
std::cout << "B::foo" << endl;
}
};
This would have to be the version that does compile, as the foo function signatures match exactly. However, when I compile it with GCC I get the following errors:
testme.cc:11: error: cannot declare member function `BaseClass::foo' within `A'
testme.cc:20: error: cannot declare member function `BaseClass::foo' within `B'
It might have been that GCC is wrong about the standard, so I tried it with Comeau's online compiler. Their compiler is extremely standards compliant (read the opinion of the Slashdot crowd in this article) and so far it has never failed me with regard to standards compliance. I always use this compiler to test if something doesn't compile because of a compiler quirk or because the code is simply incorrect.
Comeau gives me:
"ComeauTest.c", line 10: error: qualified name is not allowed
virtual void BaseClass::foo(int n) // Explicit override
^
"ComeauTest.c", line 19: error: qualified name is not allowed
virtual void BaseClass::foo(int n)
I also tried a similar trick using a "using" declaration. However, I can only write
class A : public BaseClass
{
public:
using BaseClass::foo;
virtual void foo(int n) {
}
};
This makes sure that a function named "foo" exists in BaseClass (which is already pretty useful). However, it does not make sure that the signatures match. Does anyone know the correct syntax for something like this:
class A : public BaseClass
{
public:
using BaseClass::foo(int);
virtual void foo(int n) {
}
};
Because if I had a choice, something like this would be the nicest solution! It doesn't compile on either GCC or Comeau, however. Apparently the standard doesn't allow "using" a specific overload of a function, or I'm missing some important syntax here.
Anyway, thanks for the idea! Maybe they'll allow this one day, and then I'll gladly do it this way.
-- Bart
|
|
|
|
 |
|
 |
My fault
Sorry, i don´t have access to the latest C++ standard.
It compiled fine under VC 7.1 and BCB X preview compiler (which i thought is based on the same frontend as Comeau - EDG ?) so i assumed explicit override is supported by the standard.
Anyway, i think too - they should add this to the standard.
-- Andre
|
|
|
|
 |