views:

86

answers:

2

I'm seeing what I believe to be an erroneous/incorrect compiler error using the Visual Studio 2010 compiler. I'm in the process of up-porting our codebase from Visual Studio 2005 and I ran across a construct that was building correctly before but now generates a C2248 compiler error.

Obviously, the code snippet below has been generic-ized, but it is a compilable example of the scenario. The ObjectPtr<T> C++ template comes from our codebase and is the source of the error in question. What appears to be happening is that the compiler is generating a call to the copy constructor for ObjectPtr<T> when it shouldn't (see my comment block in the SomeContainer::Foo() method below). For this code construct, there is a public cast operator for SomeUsefulData * on ObjectPtr<SomeUsefulData> but it is not being chosen inside the true expression if the ?: operator. Instead, I get the two errors in the block quote below.

Based on my knowledge of C++, this code should compile. Has anyone else seen this behavior? If not, can someone point me to a clarification of the compiler resolution rules that would explain why it's attempting to generate a copy of the object in this case?

Thanks in advance,
Dylan Bourque

Visual Studio build output:

c:\projects\objectptrtest\objectptrtest.cpp(177): error C2248: 'ObjectPtr::ObjectPtr' : cannot access private member declared in class 'ObjectPtr'
with
[ T=SomeUsefulData ]
c:\projects\objectptrtest\objectptrtest.cpp(25) : see declaration of 'ObjectPtr::ObjectPtr'
with
[ T=SomeUsefulData ]
c:\projects\objectptrtest\objectptrtest.cpp(177): error C2248: 'ObjectPtr::ObjectPtr' : cannot access private member declared in class 'ObjectPtr'
with
[ T=SomeUsefulData ]
c:\projects\objectptrtest\objectptrtest.cpp(25) : see declaration of 'ObjectPtr::ObjectPtr'
with
[ T=SomeUsefulData ]


Below is a minimal, compilable example of the scenario:

#include <stdio.h>
#include <tchar.h>
template<class T>
class ObjectPtr {
public:
   ObjectPtr<T> (T* pObj = NULL, bool bShared = false) :
      m_pObject(pObj), m_bObjectShared(bShared)
   {}
   ~ObjectPtr<T> ()
   {
      Detach();
   }
private:
   // private, unimplemented copy constructor and assignment operator
   // to guarantee that ObjectPtr<T> objects are not copied
   ObjectPtr<T> (const ObjectPtr<T>&);
   ObjectPtr<T>& operator = (const ObjectPtr<T>&);
public:
   T * GetObject ()
      { return m_pObject; }
   const T * GetObject () const
      { return m_pObject; }
   bool HasObject () const
      { return (GetObject()!=NULL); }
   bool IsObjectShared () const
      { return m_bObjectShared; }
   void ObjectShared (bool bShared)
      { m_bObjectShared = bShared; }
   bool IsNull () const
      { return !HasObject(); }
   void Attach (T* pObj, bool bShared = false)
   {
      Detach();
      if (pObj != NULL) {
         m_pObject = pObj;
         m_bObjectShared = bShared;
      }
   }
   void Detach (T** ppObject = NULL)
   {
      if (ppObject != NULL) {
         *ppObject = m_pObject;
         m_pObject = NULL;
         m_bObjectShared = false;
      }
      else {
         if (HasObject()) {
            if (!IsObjectShared())
               delete m_pObject;
            m_pObject = NULL;
            m_bObjectShared = false;
         }
      }
   }
   void Detach (bool bDeleteIfNotShared)
   {
      if (HasObject()) {
         if (bDeleteIfNotShared && !IsObjectShared())
            delete m_pObject;
         m_pObject = NULL;
         m_bObjectShared = false;
      }
   }
   bool IsEqualTo (const T * pOther) const
      { return (GetObject() == pOther); }
public:
   T * operator -> ()
      { ASSERT(HasObject()); return m_pObject; }
   const T * operator -> () const
      { ASSERT(HasObject()); return m_pObject; }
   T & operator * ()
      { ASSERT(HasObject()); return *m_pObject; }
   const T & operator * () const
      {  ASSERT(HasObject()); return (const C &)(*m_pObject); }
   operator T * ()
      { return m_pObject; }
   operator const T * () const
      { return m_pObject; }
   operator bool() const
      { return (m_pObject!=NULL); }
   ObjectPtr<T>& operator = (T * pObj)
      { Attach(pObj, false); return *this; }
   bool operator == (const T * pOther) const
      { return IsEqualTo(pOther); }
   bool operator == (T * pOther) const
      { return IsEqualTo(pOther); }
   bool operator != (const T * pOther) const
      { return !IsEqualTo(pOther); }
   bool operator != (T * pOther) const
      { return !IsEqualTo(pOther); }
   bool operator == (const ObjectPtr<T>& other) const
      { return IsEqualTo(other.GetObject()); }
   bool operator != (const ObjectPtr<T>& other) const
      { return !IsEqualTo(other.GetObject()); }
   bool operator == (int pv) const
      { return (pv==NULL)? IsNull() : (LPVOID(m_pObject)==LPVOID(pv)); }
   bool operator != (int pv) const
      { return !(*this == pv); }
private:
   T * m_pObject;
   bool m_bObjectShared;
};

// Some concrete type that holds useful data
class SomeUsefulData {
public:
   SomeUsefulData () {}
   ~SomeUsefulData () {}
};

// Some concrete type that holds a heap-allocated instance of
// SomeUsefulData
class SomeContainer {
public:
   SomeContainer (SomeUsefulData* pUsefulData)
   {
      m_pData = pUsefulData;
   }
   ~SomeContainer ()
   {
      // nothing to do here
   }
public:
   bool EvaluateSomeCondition ()
   {
      // fake condition check to give us an expression
      // to use in ?: operator below
      return true;
   }
   SomeUsefulData* Foo ()
   {
      // this usage of the ?: operator generates a C2248
      // error b/c it's attempting to call the copy
      // constructor on ObjectPtr<T>
      return EvaluateSomeCondition() ? m_pData : NULL;
      /**********[ DISCUSSION ]**********
      The following equivalent constructs compile
      w/out error and behave correctly:

      (1) explicit cast to SomeUsefulData* as a comiler hint
      return EvaluateSomeCondition() ? (SomeUsefulData *)m_pData : NULL;

      (2) if/else instead of ?:
      if (EvaluateSomeCondition())
         return m_pData;
      else
         return NULL;

      (3) skip the condition check and return m_pData as a
          SomeUsefulData* directly
      return m_pData;
      **********[ END DISCUSSION ]**********/
   }
private:
   ObjectPtr<SomeUsefulData> m_pData;
};

int _tmain(int argc, _TCHAR* argv[])
{
   return 0;
}
+2  A: 

The constructors and destructors should not have the class's template parameters:

   ObjectPtr(T* pObj = NULL, bool bShared = false) :
      m_pObject(pObj), m_bObjectShared(bShared)
   {}

(note the lack of <T>)

But I think this is unrelated. See my answer below...

rlbond
very nice catch, +1
Doug T.
+1  A: 

I don't own an actual copy of the C++ standard, but from this draft, page 102-103, a statement is ill-formed if, for expression ? E1 : E2, with types T1 and T2, and if T1 and T2 do not have an inheritance relationship, and one is an rvalue, then,

Using this process, it is determined whether the second operand can be converted to match the third operand, and whether the third operand can be converted to match the second operand. If both can be converted, or one can be converted but the conversion is ambiguous, the program is ill-formed. If neither can be converted, the operands are left unchanged and further checking is performed as described below. If exactly one conversion is possible, that conversion is applied to the chosen operand and the converted operand is used in place of the original operand for the remainder of this section.

This seems to suggest that your statement is ill-formed (since you have a non-explicit constructor of ObjectPtr as well as operator T*), but as I said I don't have the actual standard.

rlbond
I always assumed (incorrectly as I see now) that the statement `condition ? E1 : E2` was semantically identical to `if (condition) { E1 } else { E2 }`. I was not aware of the type matching between `E1` and `E2`. Thanks for the info.I had already worked around the compiler error by restructuring the code, but it's still good to know the *correct* way to do it.
Dylan Bourque