Re: [PATCH v2 1/2] pkl: codegen: make struct_{constructor,deintegrator} exception-safe

[Jose E. Marchesi]

> @@ -2174,9 +2178,15 @@
>          .let @itype = PKL_AST_TYPE_S_ITYPE (@type_struct)
>          .let @uint64_type = pkl_ast_make_integral_type (PKL_PASS_AST, 64, 0)
>          .e zero_extend_64 @itype
> -        regvar $ival
> +        ;; If constraint violation exception happens during the
> +        ;; construction of deintegrated struct, this is the right
> +        ;; place to restore the stack.
> +        push PVM_E_CONSTRAINT
> +        pushe .constraint_failed

I agree the dup below is necessary in case the caller to
struct_deintegrator catches an exception and needs the stack restored
without the argument trashed.

But why is this intermediate `pushe' needed at all?  Just let the
exception fly thru.

> +        dup                     ; IVAL IVAL
> +        regvar $ival            ; IVAL
>          ;; This is the offset argument to the mksct instruction below.
> -        push ulong<64>0         ; OFF
> +        push ulong<64>0         ; IVAL OFF
>          ;; Iterate over the struct named fields creating triplets for the
>          ;; fields, whose value is extracted from IVAL.  We know that
>          ;; IVAL has the same width than the struct fields all combined.
> @@ -2207,30 +2217,32 @@
>   .c     }
>          .let #bit_offset = pvm_make_int (bit_offset, 32)
>          ;; Extract the value for this field from IVAL
> -        pushvar $ival           ; IVAL
> +        pushvar $ival           ; IVAL IVAL
>          .e deint_extract_field_value @uint64_type, @itype, @field_type, 
> #bit_offset
>          ;; Create the triplet with the converted value.
>          .let #field_name = pvm_make_string (PKL_AST_IDENTIFIER_POINTER 
> (@type_field_name))
>          .let #field_offset = pvm_make_ulong (bit_offset, 64)
> -        push #field_offset      ; CVAL OFFSET
> -        push #field_name        ; CVAL OFFSET NAME
> -        rot                     ; OFFSET NAME CVAL
> +        push #field_offset      ; IVAL CVAL OFFSET
> +        push #field_name        ; IVAL CVAL OFFSET NAME
> +        rot                     ; IVAL OFFSET NAME CVAL
>   .c     bit_offset += field_type_size;
>   .c     i++;
>   .c }
> -                                ; OFF [TRIPLETS...]
> +                                ; IVAL OFF [TRIPLETS...]
>          .let #nfields = pvm_make_ulong (i, 64)
> -        push ulong<64>0         ; OFF [TRIPLETS...] NMETHODS
> -        push #nfields           ; OFF [TRIPLETS...] NMETHODS NFIELDS
> +        push ulong<64>0         ; IVAL OFF [TRIPLETS...] NMETHODS
> +        push #nfields           ; IVAL OFF [TRIPLETS...] NMETHODS NFIELDS
>    .c    PKL_GEN_PUSH_SET_CONTEXT (PKL_GEN_CTX_IN_TYPE);
>    .c    PKL_PASS_SUBPASS (@type_struct);
>    .c    PKL_GEN_POP_CONTEXT;
> -                                ; OFF [TRIPLETS...] NMETHODS NFIELDS TYPE
> +                                ; IVAL OFF [TRIPLETS...] NMETHODS NFIELDS 
> TYPE
>          mksct
>    .c    PKL_GEN_PUSH_SET_CONTEXT (PKL_GEN_CTX_IN_CONSTRUCTOR);
>    .c    PKL_PASS_SUBPASS (@type_struct);
>    .c    PKL_GEN_POP_CONTEXT;
> -                                ; SCT
> +                                ; IVAL SCT
> +        pope
> +        nip                     ; SCT
>          ;; At this point the anonymous fields in the struct created above are
>          ;; all zero.  This is because we coudln't include them in the 
> argument
>          ;; to the struct constructor.  So now we have to iterate over the
> @@ -2269,6 +2281,8 @@
>   .c }
>          popf 1
>          return
> +.constraint_failed:
> +        raise
>          .end
>  
>  ;;; RAS_MACRO_COMPLEX_LMAP @type #writer