[petsc-dev] inconsistency for its own sake?

[Matthew Knepley]

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

> 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.
>

I don't like them either, but I agree with you that this is a technical
problem that could be solved, just not by us.

  Matt


> 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().
>>
>>
>


-- 
What most experimenters take for granted before they begin their
experiments is infinitely more interesting than any results to which their
experiments lead.
-- Norbert Wiener
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20130111/f21e7ac4/attachment.html>