<br><br><div class="gmail_quote">On Sat, Sep 29, 2012 at 12:16 PM, Barry Smith <span dir="ltr"><<a href="mailto:bsmith@mcs.anl.gov" target="_blank">bsmith@mcs.anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im"><br>
On Sep 29, 2012, at 11:03 AM, Jed Brown <<a href="mailto:jedbrown@mcs.anl.gov">jedbrown@mcs.anl.gov</a>> wrote:<br>
<br>
> On Fri, Sep 28, 2012 at 9:24 PM, Barry Smith <<a href="mailto:bsmith@mcs.anl.gov">bsmith@mcs.anl.gov</a>> wrote:<br>
><br>
>   I don't like having two typedefs<br>
><br>
>   I tried<br>
><br>
> typedef const char* VecType;<br>
> PETSC_EXTERN PetscErrorCode VecSetType(Vec, const VecType);<br>
><br>
> ^^^ Note that this const is superfluous.<br></div></blockquote><div>Why is it superfluous?  Isn't the second argument type const char* const this way?</div><div><br></div><div>Dmitry. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im">
<br>
</div>    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<br>
<span class="HOEnZb"><font color="#888888"><br>
   Barry<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
><br>
> PETSC_EXTERN PetscErrorCode VecGetType(Vec, VecType *);<br>
><br>
> then everything compiled fine except I need to change in a bunch of places<br>
><br>
> ierr = PetscStrallocpy(ctype,(char**)&da->vectype);CHKERRQ(ierr);<br>
><br>
>   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.<br>
><br>
><br>
><br>
>    Barry<br>
><br>
><br>
><br>
><br>
> On Sep 28, 2012, at 6:16 PM, Karl Rupp <<a href="mailto:rupp@mcs.anl.gov">rupp@mcs.anl.gov</a>> wrote:<br>
><br>
> > Hi,<br>
> ><br>
> >>     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.<br>


> >><br>
> >>     I view this problem as a slight "flaw" in typedef, but perhaps C typedef has a solution?<br>
> >><br>
> >>    Barry<br>
> >><br>
> ><br>
> > okay, this explains the 'why', thanks. :-)<br>
> ><br>
> > 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:<br>
> ><br>
> > #include "stdio.h"<br>
> > #include "stdlib.h"<br>
> ><br>
> > #define VecType   char*<br>
> > #define VECSHARED "shared"<br>
> ><br>
> > int SetType(const VecType v){<br>
> > printf("%c\n", v[0]);<br>
> > return 0;<br>
> > }<br>
> > int GetType(const VecType* v){<br>
> >  static char bla[] = "bla";<br>
> >  *v = bla;<br>
> >  return 1;<br>
> > }<br>
> ><br>
> > int main(int argc, char **argv){<br>
> >  const char test1[] = "test1";<br>
> >  char test2[] = "test2";<br>
> >  const char* test3 = "test3";<br>
> >  VecType test4 = (VecType)malloc(5*sizeof(char));<br>
> >  const VecType test5 = test4;<br>
> ><br>
> >  test4[0] = 'a';<br>
> ><br>
> >  printf("-- set --\n");<br>
> >  SetType(test1);<br>
> >  SetType(test2);<br>
> >  SetType(test3);<br>
> >  SetType(test4);<br>
> >  SetType(test5);<br>
> >  SetType(VECSHARED);<br>
> ><br>
> >  printf("-- get --\n");<br>
> >  /*GetType(&test4);  <-- does not compile with #define<br>
> >  printf("%c\n", test4[0]); */<br>
> >  GetType(&test5);<br>
> >  printf("%c\n", test5[0]);<br>
> ><br>
> >  return 0;<br>
> > }<br>
> ><br>
> > I note the following<br>
> > ==> It compiles cleanly<br>
> > ==> 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<br>


> >  const VecType type;<br>
> >  GetType(&type);<br>
> > (see src/vec/vec/impls/nest/vecnest.c) or even the undesired<br>
> >  const char *tname;<br>
> >  *ierr = GetType(&tname);<br>
> > (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?).<br>


> ><br>
> ><br>
> > Now let's consider the options we have:<br>
> > a) typedef char* VecType;  no longer compiles cleanly, as Barry pointed out.<br>
> > b) typedef const char* VecType;  imposes usability restrictions and would hence break a lot of user-code.<br>
> > 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...<br>
> > d) Consider the following modification: Instead of a single preprocessor-define, use<br>
> >  typedef       char* VecType;<br>
> >  typedef const char* VecType_;<br>
> > and use the function headers<br>
> >  int SetType(const VecType_ v);<br>
> >  int GetType(VecType * v)<br>
> > This allows for a more intuitive const-correctness, i.e.<br>
> >  VecType type; GetType(&type);<br>
> > 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<br>
> >  VecType_ new_type = "shared"; SetType(new_type);<br>
> > the code remains valid.<br>
> ><br>
> > So, even if option d) is still not the cleanest solution we could dream of, I think it is preferable over the current state.<br>
> ><br>
> > Best regards,<br>
> > Karli<br>
> ><br>
> ><br>
><br>
><br>
<br>
</div></div></blockquote></div><br>