[petsc-dev] inconsistency for its own sake?

[Jed Brown]

Ah, Dependency versus Duplication. I blame the lack of transparency on the
editors, which could display the definition of the typedef instead of
having to jump to it. I like that the typedefs allow us to document the
function in one place and that changing arguments (hopefully only while
working out the initial implementation, but whenever it's deemed necessary)
is more reliable. Since dynamic functions are cast to void(*)(void), the
compiler may not be able to check that types match. Also, ability to grep
for any place that a given callback is manipulated.

Pros of typedef:
* typedef is, in principle, "safer" for developers
* typedef helps avoid duplication in documentation; maintains symmetry
between Get() and Set()
* typedef uses simpler type notation (nice for less experienced C
programmers)

Cons of typedef:
* typedef is less transparent when reading code
* information is more scattered (harder for people that don't have the
source code indexed and don't know where to find definitions)
* because every usage does not need to be updated, it's hard to determine
consequences of a change when reading diff

I'll stop using function pointer typedefs if you don't like them.

On Fri, Jan 11, 2013 at 10:26 AM, Barry Smith <bsmith at mcs.anl.gov> wrote:

>
>    The thing I don't like about typedef's for function pointers is that
> they do not display the calling sequencing in the code
>
>   For example  from
>
> > typedef struct _n_TSDM *TSDM;
> > struct _n_TSDM {
> >   TSRHSFunction rhsfunction;
>
>   or
>
>    PetscErrorCode TSSetRHSFunction(TS ts, TSRHSFunction rhsfunction)
>
>   I have no freaking idea what the calling sequence of rhsfunction is. but
> with
>
> > typedef struct _n_TSDM *TSDM;
> > struct _n_TSDM {
> >   PetscErrorCode (*TSRHSFunction)(TS,PetscReal,Vec,Vec,void*);
>
>  or
>
>     PetscErrorCode TSSetRHSFunction(TS ts, PetscErrorCode
> (*TSRHSFunction)(TS,PetscReal,Vec,Vec,void*);
>
> boom I know exactly the calling sequence.
>
>    Maybe there is some way we can do manual pages for these beasties and
> consistency of names in different locations so we get the best of both
> worlds?
>
>    Barry
>
>
>
> On Nov 25, 2012, at 3:58 AM, Jed Brown <jedbrown at mcs.anl.gov> wrote:
>
> > On Sun, Nov 25, 2012 at 4:42 AM, Barry Smith <bsmith at mcs.anl.gov> wrote:
> >
> > typedef struct _n_KSPDM *KSPDM;
> > struct _n_KSPDM {
> >   PetscErrorCode (*computeoperators)(KSP,Mat,Mat,MatStructure*,void*);
> >   PetscErrorCode (*computerhs)(KSP,Vec,void*);
> >   PetscErrorCode (*computeinitialguess)(KSP,Vec,void*);
> >
> > typedef struct _n_SNESDM *SNESDM;
> > struct _n_SNESDM {
> >   PetscErrorCode (*computefunction)(SNES,Vec,Vec,void*);
> >   PetscErrorCode (*computegs)(SNES,Vec,Vec,void*);
> >   PetscErrorCode
> (*computejacobian)(SNES,Vec,Mat*,Mat*,MatStructure*,void*);
> >
> >   /* objective */
> >   PetscErrorCode (*computeobjective)(SNES,Vec,PetscReal*,void*);
> >
> >
> > typedef struct _n_TSDM *TSDM;
> > struct _n_TSDM {
> >   TSRHSFunction rhsfunction;
> >   TSRHSJacobian rhsjacobian;
> >
> >   TSIFunction ifunction;
> >   TSIJacobian ijacobian;
> >
> > The PETSc style guide says  to avoid typedef of function pointers unless
> there is a damn good reason to use them (or it should);
> >
> > Can we revisit this choice in the interest of writing one man page that
> documents the assumptions that one can make about a callback routine? I
> hate duplicating the same information in TS{Set,Get}IFunction() and
> DMTS{Set,Get}IFunction(). Also, if a typedef is used everywhere, we can
> more reliably find all places that depend on it. (using grep or M-x
> gtags-find-symbol).
> >
> > but why use them for TS but not for SNES or KSP functions?
> >
> > Because they were already defined for use with
> TSSetIFunction()/TSGetIFunction().
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20130111/62491f00/attachment.html>