[petsc-dev] Preprocessor hell: #define VecType

[Dmitry Karpeev]

On Sat, Sep 29, 2012 at 12:16 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:

>
> On Sep 29, 2012, at 11:03 AM, Jed Brown <jedbrown at mcs.anl.gov> wrote:
>
> > On Fri, Sep 28, 2012 at 9:24 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
> >
> >   I don't like having two typedefs
> >
> >   I tried
> >
> > typedef const char* VecType;
> > PETSC_EXTERN PetscErrorCode VecSetType(Vec, const VecType);
> >
> > ^^^ Note that this const is superfluous.
>
Why is it superfluous?  Isn't the second argument type const char* const
this way?

Dmitry.

>
>     But is it harmful? (Will it every produce a compiler complaint?)   It
> is nice to have const on sets and not a const on gets
>
>    Barry
>
>
> >
> > PETSC_EXTERN PetscErrorCode VecGetType(Vec, VecType *);
> >
> > then everything compiled fine except I need to change in a bunch of
> places
> >
> > ierr = PetscStrallocpy(ctype,(char**)&da->vectype);CHKERRQ(ierr);
> >
> >   I can live with this interface (that is requiring some casting when
> manipulating as strings). Does anyone see problems with this approach?  I
> know I was opposed to this before but I've come around.
> >
> >
> >
> >    Barry
> >
> >
> >
> >
> > On Sep 28, 2012, at 6:16 PM, Karl Rupp <rupp at mcs.anl.gov> wrote:
> >
> > > Hi,
> > >
> > >>     The problem is we/people may want to "build" XXXType values on
> the fly with string operations like strcpy, strcat etc. You cannot do that
> into a const char*, thus we/people would have to declare the place they
> build things as char* instead of XXXType and that is "unnatural".  Keep
> digging; we all agree with you that it would be good to get rid of the
> #define.
> > >>
> > >>     I view this problem as a slight "flaw" in typedef, but perhaps C
> typedef has a solution?
> > >>
> > >>    Barry
> > >>
> > >
> > > okay, this explains the 'why', thanks. :-)
> > >
> > > I've played a bit with the options we have and finally came up with
> the following snippet, which  resembles the XYZSetType() and XYZGetType()
> functions currently in use:
> > >
> > > #include "stdio.h"
> > > #include "stdlib.h"
> > >
> > > #define VecType   char*
> > > #define VECSHARED "shared"
> > >
> > > int SetType(const VecType v){
> > > printf("%c\n", v[0]);
> > > return 0;
> > > }
> > > int GetType(const VecType* v){
> > >  static char bla[] = "bla";
> > >  *v = bla;
> > >  return 1;
> > > }
> > >
> > > int main(int argc, char **argv){
> > >  const char test1[] = "test1";
> > >  char test2[] = "test2";
> > >  const char* test3 = "test3";
> > >  VecType test4 = (VecType)malloc(5*sizeof(char));
> > >  const VecType test5 = test4;
> > >
> > >  test4[0] = 'a';
> > >
> > >  printf("-- set --\n");
> > >  SetType(test1);
> > >  SetType(test2);
> > >  SetType(test3);
> > >  SetType(test4);
> > >  SetType(test5);
> > >  SetType(VECSHARED);
> > >
> > >  printf("-- get --\n");
> > >  /*GetType(&test4);  <-- does not compile with #define
> > >  printf("%c\n", test4[0]); */
> > >  GetType(&test5);
> > >  printf("%c\n", test5[0]);
> > >
> > >  return 0;
> > > }
> > >
> > > I note the following
> > > ==> It compiles cleanly
> > > ==> GetType() expects a pointer to pointer to const. Well, this is
> probably not overly intuitive, as this suggests that the internal
> representation is a pointer to const char, while it is actually a char*
> (see petsc-private/petscimpl.h if I'm not mistaken...). Moreover, in order
> to call the getter function, one needs to write
> > >  const VecType type;
> > >  GetType(&type);
> > > (see src/vec/vec/impls/nest/vecnest.c) or even the undesired
> > >  const char *tname;
> > >  *ierr = GetType(&tname);
> > > (see src/vec/vec/interface/ftn-custom/zvecregf.c). Hence, while people
> will usually expect to instantiate a VecType and pass it to GetType(), they
> have to actually instantiate a 'const VecType' and pass it to GetType in
> order to get it modified appropriately (huh?).
> > >
> > >
> > > Now let's consider the options we have:
> > > a) typedef char* VecType;  no longer compiles cleanly, as Barry
> pointed out.
> > > b) typedef const char* VecType;  imposes usability restrictions and
> would hence break a lot of user-code.
> > > c) Keep it as-is. This compiles cleanly, yet the GetType() issue is
> not intuitive. Also, the Sword of Damocles (here: the preprocessor define)
> is a constant threat to other code...
> > > d) Consider the following modification: Instead of a single
> preprocessor-define, use
> > >  typedef       char* VecType;
> > >  typedef const char* VecType_;
> > > and use the function headers
> > >  int SetType(const VecType_ v);
> > >  int GetType(VecType * v)
> > > This allows for a more intuitive const-correctness, i.e.
> > >  VecType type; GetType(&type);
> > > and gets rid of the C preprocessor issue. Also, the reason for the
> trailing underscore can be explained rather nicely in the documentation and
> should not cause confusion: Even if a user writes
> > >  VecType_ new_type = "shared"; SetType(new_type);
> > > the code remains valid.
> > >
> > > So, even if option d) is still not the cleanest solution we could
> dream of, I think it is preferable over the current state.
> > >
> > > Best regards,
> > > Karli
> > >
> > >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20120929/37aff8d6/attachment.html>