[MOAB-dev] Incomplete types versus void * in ITAPS interfaces

[Jed Brown]

On Sun 2008-09-07 16:21, Tim Tautges wrote:
> I don't think that's quite what I had in mind, based on Jed's question.  
> I see the struct pointers still as proxies for what the handles really  
> are down in the implementations; thus, my question about whether we  
> really need to define the private structure or just reference it.  The  
> only thing I see us doing with these *'s is casting them to what our  
> handles really are, in the implementation.  Using private struct *'s  
> simply allows us to differentiate between the various types of handles,  
> rather than using void* for all of them.  That's what gives us the  
> compile time type checking.

Exactly, you never need to define the private structure because you
never dereference it.  If you try to dereference it, the compiler will
throw an error saying you are dereferencing an incomplete type instead
of an error saying you are dereferencing void.  Of course if it is
convenient for your implementation, you are welcome to define the
struct/class internally (then you can dereference it internally).  Or
you can just cast it as is currently done.

Onkar, what is the benefit of the inheritance?  You can already
determine type/topology through the interface so print_EntInfo() is
easily defined without any inheritance.  Since you will normally have a
plain iBase_EntityHandle, how do you get a more specific type?  For
instance, suppose I call iMesh_getEntities(...,iBase_VERTEX,...).  Now I
want to specialize them so I can call some function which requires an
iBase_VertexHandle.  To avoid casting the pointer to a more specialized
type myself (this exposes implementation at some level), the interface
would need a function to do this.  (In debugging mode, it should
probably confirm that the types really do match so the cast makes
sense.)  I'm not sure if this level of extra type-checking is
beneficial.


The following is a tangent to explain how to map the inheritance to C,
if it was really thought to be useful.  I'll take PETSc as an example.

      /* private */
  struct _p_PetscObject {
    struct _PetscOps *bops; /* base class virtual functions */
    /* other parent class stuff */
  };
  struct _p_Mat {
    struct _p_PetscObject hdr;
    struct _MatOps *ops; /* virtual functions for Mat interface */
    ... /* Mat specific stuff */
  };
  
      /* public */
  typedef struct _p_PetscObject *PetscObject;
  typedef struct _p_Mat *Mat;
  
      /* user */
  Mat A;
  Vec b;
  PetscObjectCompose((PetscObject)A,"special vector",(PetscObject)b);
  

Since the first part of struct _p_Mat consists of the fields in
_p_PetscObject, you get inheritance with full virtual functions.  You
cannot do protected inheritance per-se since the Mat implementation
needs to be able to see _p_PetscObject.  However, you can have a
convention that you never dereference Mat.hdr, instead operating through
the PetscObject interface (this is the case in PETSc).  Note that we
never cast the other way, that is, you never see

  PetscObject obj;
  ... /* somehow obj now points to a matrix */
  MatMult((Mat)obj,...);

since this is generally not safe (and exposes implementation at least to
the extent that PetscObject and Mat are just pointers).

Our case is much lighter weight, but if you want inheritance, we can have

      /* interface */
  typedef struct iBase_EntityHandle_Private *iBase_EntityHandle;
  typedef struct iBase_VertexHandle_Private *iBase_VertexHandle;

      /* user */
  iBase_EntityHandle *ents;
  iBase_VertexHandle *verts;
  iMesh_getEntities(...,iBase_VERTEX,...,&ents,...);
  iBase_castToVtxArr(...,ents,...,&verts,...);
  iMesh_setVtxArrCoords(mesh,verts,...,coords,...);
  iMesh_setArrData(mesh,(iBase_EntityHandle*)verts,...,data,...); // also safe

This is all `safe' but I'm not sure it's worth it.  I think it might be
hard to avoid ever needing to cast the other way (which feels like a bad
thing to do).

To come back to Onkar's suggestion, in C it would be spelled

  print_VertexInfo(iBase_VertexHandle,...);
  print_RegionInfo(iBase_RegionHandle,...);

There isn't really more power in the C++ version (just cosmetics and you
can use templates to generate code for many specialized cases) since the
type needs to be statically determined at compile-time, hence it is
actually less flexible than

  print_EntInfo(iBase_EntityHandle,...)

which just calls iMesh_getEntType/Topo().  To get this version to do
something special without calling iMesh_getEntType/Topo() we need
virtual functions which means storing an extra pointer for every entity
(in C or C++).

Jed
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <https://lists.mcs.anl.gov/mailman/private/moab-dev/attachments/20080908/fe5591d1/attachment.pgp>