4

The following declaration adds a couple of operators for compilation in a C++ file. The definition is included in both C and C++ files.

PC-Lint is reporting Error 114: Inconsistent structure declaration for tag 'Rect' but I am certain it is safe.

I'm compiling with Visual Studio 2008.

edit - adding the explanation I sent to my client

With regards to the Rect issue; how does knowing that the struct is the same size in C and C++ eliminate the suspicion of 'undefined behaviour'.

Undefined behaviour occurs if the actual location of fields in the data structure varies according to compilation.

You have to think of all member variable access as ultimately resolving to a pointer, calculated by a pointer to the beginning of the object storage plus an offset depending on what is in that structure.

Packing and data alignment settings influence the value of the offset.

The compiler is allowed to re-order types for optimal access - it is undefined behaviour to assume that just because you declared two members in a given order that they are actually stored in that order. The only thing that declaration order guarantees is initialisation, copying and destruction order.

However, when you are talking about C and C++ compilation of a given structure within the same compiler, with the same offset settings, the chance of actual reordering is effectively zero.

Thus the only thing we need worry about is any difference in the offset of fields.

For a structure containing a simple 4 short integers, simply confirming that the C version and the C++ version are the same size guarantees that their offsets are all the same. To be more careful we can also check that the structure size = 4*sizeof(short).

I think it's worth adding those checks but once that is done, there's no need to refactor the code as would be required to use separate types in C and C++ (or to move the functions being used out into free functions).

/**
Mac-compatible rectangle type with some operators added for C++ use.
@ingroup QuickdrawPort
*/
struct Rect {
    short                           top;
    short                           left;
    short                           bottom;
    short                           right;

#ifdef __cplusplus
    Rect(short _top=0, short _left=0, short _bottom=0, short _right=0) :
        top(_top),
        left(_left),
        bottom(_bottom),
        right(_right)
    {}
    #ifdef _WINNT_  // WinDef.h has been included
        const Rect& operator=(const tagRECT& rhs) {
            top = short(rhs.top);
            left = short(rhs.left);
            bottom = short(rhs.bottom);
            right = short(rhs.right);
            return *this;
        }

        operator tagRECT() const {
            tagRECT lhs;
            lhs.top = top;
            lhs.left = left;
            lhs.bottom = bottom;
            lhs.right = right;
            return lhs;
        }

    #endif// _WINNT_
    short height() const { return bottom - top; }
    short width() const { return right - left; }
    bool empty() const { return right==left || bottom==top; }

    bool operator==(const Rect& rhs) const {
        return top == rhs.top &&
            left == rhs.left && 
            bottom == rhs.bottom &&
            right == rhs.right;
    }
#endif  
};
#ifndef __cplusplus
typedef struct Rect Rect;
#endif
cafce25
  • 15,907
  • 4
  • 25
  • 31
Andy Dent
  • 17,578
  • 6
  • 88
  • 115
  • 2
    Could it be related to the fact that simple struct is a "pod" (plain-old-data), while struct with a constructor is not? – CygnusX1 Feb 28 '11 at 11:02

1 Answers1

7

Am I correct to assume that the header file containing this definition is included in several translation units, some of which are compiled as C++, and some -- as plain C?

If it is true, then you have an ODR violation, which, according to the C++ standard, invokes undefined behavior.

There are two ways to deal with it.

  1. Ignore this instance of undefined behavior, provided that you know how exactly it manifests on platform(s) you need to support. (Note that undefined behavior can manifest as a correctly working program). To tell the truth, with most compilers you will not have a problem here. However, you must understand that this code works by happenstance, not by the law. Compilers are permitted to use different layout for classes with member functions than without them. (I'm willing to bet that this code will break on CINT, for example).

  2. Eliminate undefined behavior. I'd suggest you to go this way. There are several possibilities to do it. For example, you can inherit "C++ Rect" from "C Rect", keeping the latter as a plain struct.

atzz
  • 17,507
  • 3
  • 35
  • 35
  • Unfortunately the types are being created in plain C code and then used in C++ code which provides a Mac emulation environment. I'd have to add a lot of copying to do the inheritance approach, although I agree it will be robust. This is not general use code so I'm inclined to add a sizeof check and leave it mostly as is. – Andy Dent Feb 28 '11 at 21:09
  • @Andy Dent - I agree that in practice it shouldn't be a problem. C/C++ standards leave so much ground covered by Undefined Behavior it's almost inevitable we have to rely on implementation details here and there. – atzz Mar 01 '11 at 08:51