[petsc-dev] inconsistency for its own sake?

[Barry Smith]

On Jan 11, 2013, at 1:02 PM, Jed Brown <jedbrown at mcs.anl.gov> wrote:

> On Fri, Jan 11, 2013 at 12:58 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
> 
> 
> 
> So this works by convention that we name the argument to match the man page (e.g., SNESFunction) in all interfaces?

   Yes

> The compiler won't help to enforce that consistency,

   Not yet, but maybe a couple tweaks to LLVM will get us there :-)

> but it'll deal with the documentation duplication.

   Yes (and missing in some functions like SNESGetFunction() and not existing like in other places).

  Barry

>  
> 
> Is this a good middle-ground? Something wrong with it?
> 
>   Barry
> 
> > 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().
> >
> >
> 
>