Re: [PATCH] pkl: Get rid of string literals as exception names

[Jose E. Marchesi]

> Hi Mohammad.
>
>> 2022-01-27  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>>
>>      * libpoke/pvm.h (PVM_EXCEPTIONS): New macro.
>>      * libpoke/pvm.jitter (wrapped-globals): Add `pvm_exception_names`.
>>      (PVM_RAISE_DFL): Use `pvm_exception_names` instead of string
>>      literals.
>>      (late-c): Define `pvm_exception_names`.
>> ---
>>
>> Hello, Jose.
>>
>> With this patch, I declare ARMv7 as a "supported" platform :)
>
> Related to this: I see these comments that Luca left when he first
> turned Jitter in poke to use non-threaded mode:
>
>   # XXX make non-relocatable: see raise
>
> Luca, was it because of these string constants, or because the raising
> of the exceptions?  If the first, we can remove the comments once this
> patch gets applied.
>
>>
>>                 === poke Summary ===
>>              
>>              # of expected passes            6258
>>              # of unsupported tests          7
>>              
>>
>> Regards,
>> Mohammad-Reza
>>
>>
>>  etc/pvm-wraps-static-whitelist |  1 +
>>  libpoke/pvm.h                  | 21 +++++++++++++++++++++
>>  libpoke/pvm.jitter             |  9 ++++++++-
>>  3 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/etc/pvm-wraps-static-whitelist b/etc/pvm-wraps-static-whitelist
>> index 8886c0a9..5f08f93e 100644
>> --- a/etc/pvm-wraps-static-whitelist
>> +++ b/etc/pvm-wraps-static-whitelist
>> @@ -12,6 +12,7 @@ pvm_literal_invalid_class
>>  pvm_literal_eindex
>>  pvm_literal_notmappable
>>  pvm_literal_newline
>> +pvm_exception_names
>>  pvm_memcpy
>>  pvm_strlen
>>  pvm_strcpy
>> diff --git a/libpoke/pvm.h b/libpoke/pvm.h
>> index 4baee433..0980675e 100644
>> --- a/libpoke/pvm.h
>> +++ b/libpoke/pvm.h
>> @@ -491,6 +491,27 @@ enum pvm_exit_code
>>  /* Exceptions.  These should be in sync with the exception code macros
>>     in libpoke.h, and variables and exception names, declared in pkl-rt.pk */
>>  
>> +#define PVM_EXCEPTIONS(E)    \
>> +  E(GENERIC)                 \
>> +  E(DIV_BY_ZERO)             \
>> +  E(NO_IOS)                  \
>> +  E(NO_RETURN)               \
>> +  E(OUT_OF_BOUNDS)           \
>> +  E(MAP_BOUNDS)              \
>> +  E(EOF)                     \
>> +  E(MAP)                     \
>> +  E(CONV)                    \
>> +  E(ELEM)                    \
>> +  E(CONSTRAINT)              \
>> +  E(IO)                      \
>> +  E(SIGNAL)                  \
>> +  E(IOFLAGS)                 \
>> +  E(INVAL)                   \
>> +  E(EXIT)                    \
>> +  E(ASSERT)                  \
>> +  E(OVERFLOW)                \
>> +  E(PERM)
>> +
>>  #define PVM_E_GENERIC       0
>>  #define PVM_E_GENERIC_NAME "generic"
>>  #define PVM_E_GENERIC_ESTATUS 1
>> diff --git a/libpoke/pvm.jitter b/libpoke/pvm.jitter
>> index 0cb1cf7d..8d95a155 100644
>> --- a/libpoke/pvm.jitter
>> +++ b/libpoke/pvm.jitter
>> @@ -163,6 +163,7 @@ wrapped-globals
>>    pvm_literal_notmappable
>>    pvm_literal_newline
>>    libpoke_term_if
>> +  pvm_exception_names
>>  end
>>  
>>  
>> @@ -288,7 +289,7 @@ late-header-c
>>  #define PVM_RAISE_DFL(BASE)                                           \
>>   do                                                                   \
>>   {                                                                    \
>> -   PVM_RAISE (BASE,BASE##_NAME,BASE##_ESTATUS);                        \
>> +   PVM_RAISE (BASE,pvm_exception_names[BASE],BASE##_ESTATUS);         \
>>   } while (0)
>>  
>>      /* Macros to implement different kind of instructions.  These are to
>> @@ -909,6 +910,12 @@ late-c
>>      static const char *pvm_literal_notmappable = "not mappable value";
>>      static const char *pvm_literal_newline = "\n";
>>  
>> +#define E(key) [PVM_E_##key] = PVM_E_##key##_NAME,
>> +    static const char *pvm_exception_names[] = {
>> +      PVM_EXCEPTIONS(E)
>> +    };
>> +#undef E
>> +
>>      static int
>>      pvm_nanosleep (const struct timespec *rqtp, struct timespec *rmtp)
>>      {
>
> I think the table of exception names should be in pvm.h, not in
> pvm.jitter.  And it would be better to define it in a portable way,
> without array initializers:
>
> const char *pvm_exception_names[] =
> {
>   PVM_E_GENERIC_NAME,
>   ...
> };
>
> After the release I will put all the C side of the exception information
> in a pkl-exception.def file.  But for now the above will suffice.

I just applied the patch on your behalf.
After the release we can see how to improve this.

Thanks.