A serious VC++ Compiler Bug
-
My ANSI standards have finally arrived! Erik, thanks for quoting the relevent sections. I think we are getting closer to the truth on this matter now. You quoted Section 5.2.9 clause 8, then suggested that cv-qualification is an issue here. Specifically, you said "An incomplete type is not the same cv-type as the same type that's been completed." I don't believe that's true. CV-qualification (whether something is const and/or volatile) is not determined or affected by whether a type is complete or incomplete. Section 3.9.3 clause 1 says "A type mentioned in 3.9.1 and 3.9.2 is a cv-unqualified type" and Section 3.9.2 specifies all pointer to objects, whether they are complete or otherwise. So we have a pointer to a complete object type and a pointer to an incomplete object type - their cv-qualification is identical. Therefore, Section 5.2.9 clause 8 actually supports my case now. However, I think the key to this is Section 5.4, as you quoted it. As you pointed out, the standard says that it's unspecified whether the static_cast or reinterpret_cast is used when a pointer is to an incomplete class. This is precisely the case that Nev had.
B\* GetB() const { (B\*)pA; }According to Section 5.4 clause 6, the compiler is free to implement this as:
B\* GetB() const { return static\_cast<B\*>(pA); }or
B\* GetB() const { return reinterpret\_cast<B\*>(pA); }MSVC choose the reinterpret_cast implementation according to its freedom under this Section 5.4 clause 6. Because the type is incomplete, the compiler can't use a static_cast without generating an error message and terminating the compilation. So, MS chose to use a reinterpret_cast. (I hope we all wish they'd made the opposite choice.) The problem is that with the change in the memory map between classes with virtuals and classes without, the choice to use a reinterpret_cast is a choice fraught with danger. The danger being illustrated by the problem that Nev encountered. So, do I think this is a compiler bug or not? The compiler is clearly doing what it is allowed to do by the latest standard. Like the word "weed", the word "bug" often has a subjective meaning rather than a clear objective meaning. Whether this is a bug or not depends on the intentions of the compiler's implementors. However, if I'd written the compiler, then found this problem, I'd consider:
- the danger to be too great
- the memory map decision had an unexpected implication
MS's object model goes back almost 10 years, long before they even began to standardize C++. They can't really change it at this point because too much depends on it, specifically, the COM object model is based on MS's C++ object model. Changing it would mean that you could no longer treat C++ interfaces as C++ objects. Since the interface complies with the C++ standard, it's highly unlikely that they will change it to solve a relatively minor problem. Ok, so change the way the compiler treats c-style casts, right? That too would probably break much code. Again, what price to pay for a relatively minor problem that is officially discouraged anyways. As for producing a warning, that's fine, unless of course you have code that uses this in 1000 different places and suddenly your code generates 1000 warnings for something that is not really an issue if you understand how it works.
-
MS's object model goes back almost 10 years, long before they even began to standardize C++. They can't really change it at this point because too much depends on it, specifically, the COM object model is based on MS's C++ object model. Changing it would mean that you could no longer treat C++ interfaces as C++ objects. Since the interface complies with the C++ standard, it's highly unlikely that they will change it to solve a relatively minor problem. Ok, so change the way the compiler treats c-style casts, right? That too would probably break much code. Again, what price to pay for a relatively minor problem that is officially discouraged anyways. As for producing a warning, that's fine, unless of course you have code that uses this in 1000 different places and suddenly your code generates 1000 warnings for something that is not really an issue if you understand how it works.
I never said they should change their object model (memory map, as I called it). In fact, I stated that they've probably got good reason to have it they way it is. The problem with C-style casts is that the MS compiler does it in two different ways. If the type is complete, it uses static_cast and if it's incomplete it uses reinterpret_cast. But even that's not a problem; and, as we've discovered, quite valid with the latest standards. The problem is that the combination of all these issues creates a very esoteric bug that could take ages to find (Nev lost a whole day on this). Remember, this problem only happens when all the following are conditions are met:
- the base class has no virtuals
- the derived class has virtuals
- you're casting a pointer-to-base (which actually points to a derived object) to a pointer-to-derived
- the derived class is incomplete (not yet defined) at the time of the cast
Add to this the fact that other compilers don't have such a strange way of handling the memory map in derivations and you don't have a "minor problem"; you have a very nasty problem waiting to bite you in the worst possible way. As for producing a warning. First, it only has to produce the warning in the case of a cast to an incomplete type (which is probably a rare occurrence in most cases) and, secondly, it can produce the warning at level 3 or even 4; so if you have code that you "know" is OK you can kill the warnings. Quite frankly, a simple warning is not a high price to pay. The lack of it means that developers, like Nev, may "pay" for it many times over. As for "understanding how it works"; even with C and C++ you shouldn't have to know every detail of a compiler's implementation to write or *port* code. Your "understanding [of] how it works" is only valid for this one compiler. Go to another one and you have to learn it all over again. But, of course, it's all beside the point. MS don't care about developers so they're unlikely to fix anything. (I'm still waiting for a relatively bug free IDE that I've paid for multiple times. Fortunately, Nev is about to release one of the best editors on the planet - his class browser works - and I won't have to use the MS ^%$#@ anymore!) Erik, next time you have a real bug that's caused or hidden by the poorly written products from MS, maybe you won't be defending them so strongly. :-D Russell Robinson (russellr@rootsoftware.com)
-
I never said they should change their object model (memory map, as I called it). In fact, I stated that they've probably got good reason to have it they way it is. The problem with C-style casts is that the MS compiler does it in two different ways. If the type is complete, it uses static_cast and if it's incomplete it uses reinterpret_cast. But even that's not a problem; and, as we've discovered, quite valid with the latest standards. The problem is that the combination of all these issues creates a very esoteric bug that could take ages to find (Nev lost a whole day on this). Remember, this problem only happens when all the following are conditions are met:
- the base class has no virtuals
- the derived class has virtuals
- you're casting a pointer-to-base (which actually points to a derived object) to a pointer-to-derived
- the derived class is incomplete (not yet defined) at the time of the cast
Add to this the fact that other compilers don't have such a strange way of handling the memory map in derivations and you don't have a "minor problem"; you have a very nasty problem waiting to bite you in the worst possible way. As for producing a warning. First, it only has to produce the warning in the case of a cast to an incomplete type (which is probably a rare occurrence in most cases) and, secondly, it can produce the warning at level 3 or even 4; so if you have code that you "know" is OK you can kill the warnings. Quite frankly, a simple warning is not a high price to pay. The lack of it means that developers, like Nev, may "pay" for it many times over. As for "understanding how it works"; even with C and C++ you shouldn't have to know every detail of a compiler's implementation to write or *port* code. Your "understanding [of] how it works" is only valid for this one compiler. Go to another one and you have to learn it all over again. But, of course, it's all beside the point. MS don't care about developers so they're unlikely to fix anything. (I'm still waiting for a relatively bug free IDE that I've paid for multiple times. Fortunately, Nev is about to release one of the best editors on the planet - his class browser works - and I won't have to use the MS ^%$#@ anymore!) Erik, next time you have a real bug that's caused or hidden by the poorly written products from MS, maybe you won't be defending them so strongly. :-D Russell Robinson (russellr@rootsoftware.com)
Russell, You really are determined to nail Microsoft over this, aren't you! I agree, a warning is probably a good idea (or maybe not - Erik may have a point if there are too many of these warnings in 'real world' code). We now all agree that Microsoft is correct - this is not a bug. It is also probably the only possible choice that Microsoft (or anyone else) can make. The choice to use 'reinterpret_cast' on imcomplete types not really a choice! Since the incomplete type may in fact be resolved in another '.cpp' file, the comiler at the time of compilation of the current compilation unit can NEVER know the 'true' type of the incomplete class. Therefore, a 'static_cast' will be impossible. What you are suggesting is that "if the incomplete class is actually completed further down this compilation unit, then use 'static_cast' (if the types are related) - otherwise use 'reinterpret_cast'". This simply moves the problem into a different set of circumstances. So, this means that (a) Microsoft are allowed to do what they do and (b) the implementation of the 'object Model' (I'll deal with this in a moment) gives them no alternatives. And your compariosn of VC++ to other compilers may be flawed - as far as I know, all compilers may implement this situation as a "reinterpret_cast", but because of their internal object model the effect is the same as a 'static_cast'. As far as the "Object Model/Memory Map' of C++ is concerned, 'other compilers' quite often do use the same model as Microsft have adopted. Stan Lippman points out on page 90 of his book "Inside the C++ Object Model" that the Microsoft model is now generally accepted as being the more common implementation since Multiple Inheritance was added to the language. Prior to that it was more common to place the 'vptr' at the end of an object. This 'new' model gives better efficiency and performance in multiple inheritence situations. As he says, a side effect of this change is 'a loss in C language ineteroperability'. So there's the root of this - a better model to more efficiently handle a 'new' language feature (multiple Inheritence was 'new' in Release 2.0 of C++) also causes a loss of 'C' compatibility. And as I have said, the answer is to avoid mixing the two ways of thinking - classes, inheritence and virtual functions are C++, and the old style casts are 'C'. When you mix them, you can get 'odd' behavior. Don't mix them! Personally, I'd like to see the standard 'ban' old style casts, and force a programmer to use a compiler switch (