[petsc-dev] Preprocessor hell: #define VecType

[Karl Rupp]

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