Skip to content

Commit c8b60f0

Browse files
committed
py: Make viper codegen raise proper exception (ViperTypeError) on error.
This fixes a long standing problem that viper code generation gave terrible error messages, and actually no errors on pyboard where assertions are disabled. Now all compile-time errors are raised as proper Python exceptions, and are of type ViperTypeError. Addresses issue micropython#940.
1 parent 2bb5f41 commit c8b60f0

File tree

7 files changed

+74
-38
lines changed

7 files changed

+74
-38
lines changed

py/compile.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3764,22 +3764,22 @@ mp_obj_t mp_compile(mp_parse_node_t pn, qstr source_file, uint emit_opt, bool is
37643764
case MP_EMIT_OPT_VIPER:
37653765
#if MICROPY_EMIT_X64
37663766
if (emit_native == NULL) {
3767-
emit_native = emit_native_x64_new(max_num_labels);
3767+
emit_native = emit_native_x64_new(&comp->compile_error, max_num_labels);
37683768
}
37693769
comp->emit_method_table = &emit_native_x64_method_table;
37703770
#elif MICROPY_EMIT_X86
37713771
if (emit_native == NULL) {
3772-
emit_native = emit_native_x86_new(max_num_labels);
3772+
emit_native = emit_native_x86_new(&comp->compile_error, max_num_labels);
37733773
}
37743774
comp->emit_method_table = &emit_native_x86_method_table;
37753775
#elif MICROPY_EMIT_THUMB
37763776
if (emit_native == NULL) {
3777-
emit_native = emit_native_thumb_new(max_num_labels);
3777+
emit_native = emit_native_thumb_new(&comp->compile_error, max_num_labels);
37783778
}
37793779
comp->emit_method_table = &emit_native_thumb_method_table;
37803780
#elif MICROPY_EMIT_ARM
37813781
if (emit_native == NULL) {
3782-
emit_native = emit_native_arm_new(max_num_labels);
3782+
emit_native = emit_native_arm_new(&comp->compile_error, max_num_labels);
37833783
}
37843784
comp->emit_method_table = &emit_native_arm_method_table;
37853785
#endif

py/emit.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,10 @@ extern const mp_emit_method_table_id_ops_t mp_emit_bc_method_table_delete_id_ops
172172

173173
emit_t *emit_cpython_new(void);
174174
emit_t *emit_bc_new(void);
175-
emit_t *emit_native_x64_new(mp_uint_t max_num_labels);
176-
emit_t *emit_native_x86_new(mp_uint_t max_num_labels);
177-
emit_t *emit_native_thumb_new(mp_uint_t max_num_labels);
178-
emit_t *emit_native_arm_new(mp_uint_t max_num_labels);
175+
emit_t *emit_native_x64_new(mp_obj_t *error_slot, mp_uint_t max_num_labels);
176+
emit_t *emit_native_x86_new(mp_obj_t *error_slot, mp_uint_t max_num_labels);
177+
emit_t *emit_native_thumb_new(mp_obj_t *error_slot, mp_uint_t max_num_labels);
178+
emit_t *emit_native_arm_new(mp_obj_t *error_slot, mp_uint_t max_num_labels);
179179

180180
void emit_cpython_set_max_num_labels(emit_t* emit, mp_uint_t max_num_labels);
181181
void emit_bc_set_max_num_labels(emit_t* emit, mp_uint_t max_num_labels);

py/emitnative.c

Lines changed: 55 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,10 @@ STATIC byte mp_f_n_args[MP_F_NUMBER_OF] = {
482482

483483
#endif
484484

485+
#define EMIT_NATIVE_VIPER_TYPE_ERROR(emit, ...) do { \
486+
*emit->error_slot = mp_obj_new_exception_msg_varg(&mp_type_ViperTypeError, __VA_ARGS__); \
487+
} while (0)
488+
485489
typedef enum {
486490
STACK_VALUE,
487491
STACK_REG,
@@ -505,6 +509,19 @@ typedef enum {
505509
VTYPE_BUILTIN_CAST = 0x60 | MP_NATIVE_TYPE_OBJ,
506510
} vtype_kind_t;
507511

512+
STATIC qstr vtype_to_qstr(vtype_kind_t vtype) {
513+
switch (vtype) {
514+
case VTYPE_PYOBJ: return MP_QSTR_object;
515+
case VTYPE_BOOL: return MP_QSTR_bool;
516+
case VTYPE_INT: return MP_QSTR_int;
517+
case VTYPE_UINT: return MP_QSTR_uint;
518+
case VTYPE_PTR: return MP_QSTR_ptr;
519+
case VTYPE_PTR8: return MP_QSTR_ptr8;
520+
case VTYPE_PTR16: return MP_QSTR_ptr16;
521+
case VTYPE_PTR_NONE: default: return MP_QSTR_None;
522+
}
523+
}
524+
508525
typedef struct _stack_info_t {
509526
vtype_kind_t vtype;
510527
stack_info_kind_t kind;
@@ -515,6 +532,7 @@ typedef struct _stack_info_t {
515532
} stack_info_t;
516533

517534
struct _emit_t {
535+
mp_obj_t *error_slot;
518536
int pass;
519537

520538
bool do_viper_types;
@@ -542,8 +560,9 @@ struct _emit_t {
542560
ASM_T *as;
543561
};
544562

545-
emit_t *EXPORT_FUN(new)(mp_uint_t max_num_labels) {
563+
emit_t *EXPORT_FUN(new)(mp_obj_t *error_slot, mp_uint_t max_num_labels) {
546564
emit_t *emit = m_new0(emit_t, 1);
565+
emit->error_slot = error_slot;
547566
emit->as = ASM_NEW(max_num_labels);
548567
return emit;
549568
}
@@ -571,7 +590,7 @@ STATIC void emit_native_set_native_type(emit_t *emit, mp_uint_t op, mp_uint_t ar
571590
case MP_QSTR_ptr: type = VTYPE_PTR; break;
572591
case MP_QSTR_ptr8: type = VTYPE_PTR8; break;
573592
case MP_QSTR_ptr16: type = VTYPE_PTR16; break;
574-
default: mp_printf(&mp_plat_print, "ViperTypeError: unknown type %q\n", arg2); return;
593+
default: EMIT_NATIVE_VIPER_TYPE_ERROR(emit, "unknown type '%q'", arg2); return;
575594
}
576595
if (op == MP_EMIT_NATIVE_TYPE_RETURN) {
577596
emit->return_vtype = type;
@@ -1288,7 +1307,7 @@ STATIC void emit_native_load_fast(emit_t *emit, qstr qst, mp_uint_t local_num) {
12881307
DEBUG_printf("load_fast(%s, " UINT_FMT ")\n", qstr_str(qst), local_num);
12891308
vtype_kind_t vtype = emit->local_vtype[local_num];
12901309
if (vtype == VTYPE_UNBOUND) {
1291-
mp_printf(&mp_plat_print, "ViperTypeError: local %q used before type known\n", qst);
1310+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit, "local '%q' used before type known", qst);
12921311
}
12931312
emit_native_pre(emit);
12941313
if (local_num == 0) {
@@ -1438,7 +1457,8 @@ STATIC void emit_native_load_subscr(emit_t *emit) {
14381457
break;
14391458
}
14401459
default:
1441-
mp_printf(&mp_plat_print, "ViperTypeError: can't load from type %d\n", vtype_base);
1460+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit,
1461+
"can't load from '%q'", vtype_to_qstr(vtype_base));
14421462
}
14431463
} else {
14441464
// index is not an immediate
@@ -1464,7 +1484,8 @@ STATIC void emit_native_load_subscr(emit_t *emit) {
14641484
break;
14651485
}
14661486
default:
1467-
mp_printf(&mp_plat_print, "ViperTypeError: can't load from type %d\n", vtype_base);
1487+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit,
1488+
"can't load from '%q'", vtype_to_qstr(vtype_base));
14681489
}
14691490
}
14701491
emit_post_push_reg(emit, VTYPE_INT, REG_RET);
@@ -1495,7 +1516,9 @@ STATIC void emit_native_store_fast(emit_t *emit, qstr qst, mp_uint_t local_num)
14951516
emit->local_vtype[local_num] = vtype;
14961517
} else if (emit->local_vtype[local_num] != vtype) {
14971518
// type of local is not the same as object stored in it
1498-
mp_printf(&mp_plat_print, "ViperTypeError: type mismatch, local %q has type %d but source object has type %d\n", qst, emit->local_vtype[local_num], vtype);
1519+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit,
1520+
"local '%q' has type '%q' but source is '%q'",
1521+
qst, vtype_to_qstr(emit->local_vtype[local_num]), vtype_to_qstr(vtype));
14991522
}
15001523
}
15011524

@@ -1625,7 +1648,8 @@ STATIC void emit_native_store_subscr(emit_t *emit) {
16251648
break;
16261649
}
16271650
default:
1628-
mp_printf(&mp_plat_print, "ViperTypeError: can't store to type %d\n", vtype_base);
1651+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit,
1652+
"can't store to '%q'", vtype_to_qstr(vtype_base));
16291653
}
16301654
} else {
16311655
// index is not an immediate
@@ -1666,7 +1690,8 @@ STATIC void emit_native_store_subscr(emit_t *emit) {
16661690
break;
16671691
}
16681692
default:
1669-
mp_printf(&mp_plat_print, "ViperTypeError: can't store to type %d\n", vtype_base);
1693+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit,
1694+
"can't store to '%q'", vtype_to_qstr(vtype_base));
16701695
}
16711696
}
16721697

@@ -1761,25 +1786,21 @@ STATIC void emit_native_jump(emit_t *emit, mp_uint_t label) {
17611786

17621787
STATIC void emit_native_jump_helper(emit_t *emit, bool pop) {
17631788
vtype_kind_t vtype = peek_vtype(emit, 0);
1764-
switch (vtype) {
1765-
case VTYPE_PYOBJ:
1766-
emit_pre_pop_reg(emit, &vtype, REG_ARG_1);
1767-
if (!pop) {
1768-
adjust_stack(emit, 1);
1769-
}
1770-
emit_call(emit, MP_F_OBJ_IS_TRUE);
1771-
break;
1772-
case VTYPE_BOOL:
1773-
case VTYPE_INT:
1774-
case VTYPE_UINT:
1775-
emit_pre_pop_reg(emit, &vtype, REG_RET);
1776-
if (!pop) {
1777-
adjust_stack(emit, 1);
1778-
}
1779-
break;
1780-
default:
1781-
mp_printf(&mp_plat_print, "ViperTypeError: expecting a bool or pyobj, got %d\n", vtype);
1782-
assert(0);
1789+
if (vtype == VTYPE_PYOBJ) {
1790+
emit_pre_pop_reg(emit, &vtype, REG_ARG_1);
1791+
if (!pop) {
1792+
adjust_stack(emit, 1);
1793+
}
1794+
emit_call(emit, MP_F_OBJ_IS_TRUE);
1795+
} else {
1796+
emit_pre_pop_reg(emit, &vtype, REG_RET);
1797+
if (!pop) {
1798+
adjust_stack(emit, 1);
1799+
}
1800+
if (!(vtype == VTYPE_BOOL || vtype == VTYPE_INT || vtype == VTYPE_UINT)) {
1801+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit,
1802+
"can't implicitly convert '%q' to 'bool'", vtype_to_qstr(vtype));
1803+
}
17831804
}
17841805
// For non-pop need to save the vtype so that emit_native_adjust_stack_size
17851806
// can use it. This is a bit of a hack.
@@ -2056,7 +2077,9 @@ STATIC void emit_native_binary_op(emit_t *emit, mp_binary_op_t op) {
20562077
}
20572078
emit_post_push_reg(emit, VTYPE_PYOBJ, REG_RET);
20582079
} else {
2059-
mp_printf(&mp_plat_print, "ViperTypeError: can't do binary op between types %d and %d\n", vtype_lhs, vtype_rhs);
2080+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit,
2081+
"can't do binary op between '%q' and '%q'",
2082+
vtype_to_qstr(vtype_lhs), vtype_to_qstr(vtype_rhs));
20602083
emit_post_push_reg(emit, VTYPE_PYOBJ, REG_RET);
20612084
}
20622085
}
@@ -2302,7 +2325,9 @@ STATIC void emit_native_return_value(emit_t *emit) {
23022325
vtype_kind_t vtype;
23032326
emit_pre_pop_reg(emit, &vtype, REG_RET);
23042327
if (vtype != emit->return_vtype) {
2305-
mp_printf(&mp_plat_print, "ViperTypeError: incompatible return type\n");
2328+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit,
2329+
"return expected '%q' but got '%q'",
2330+
vtype_to_qstr(emit->return_vtype), vtype_to_qstr(vtype));
23062331
}
23072332
}
23082333
} else {
@@ -2320,7 +2345,7 @@ STATIC void emit_native_raise_varargs(emit_t *emit, mp_uint_t n_args) {
23202345
vtype_kind_t vtype_exc;
23212346
emit_pre_pop_reg(emit, &vtype_exc, REG_ARG_1); // arg1 = object to raise
23222347
if (vtype_exc != VTYPE_PYOBJ) {
2323-
mp_printf(&mp_plat_print, "ViperTypeError: must raise an object\n");
2348+
EMIT_NATIVE_VIPER_TYPE_ERROR(emit, "must raise an object");
23242349
}
23252350
// TODO probably make this 1 call to the runtime (which could even call convert, native_raise(obj, type))
23262351
emit_call(emit, MP_F_NATIVE_RAISE);

py/modbuiltins.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,9 @@ STATIC const mp_map_elem_t mp_module_builtins_globals_table[] = {
703703
{ MP_OBJ_NEW_QSTR(MP_QSTR_UnicodeError), (mp_obj_t)&mp_type_UnicodeError },
704704
#endif
705705
{ MP_OBJ_NEW_QSTR(MP_QSTR_ValueError), (mp_obj_t)&mp_type_ValueError },
706+
#if MICROPY_EMIT_NATIVE
707+
{ MP_OBJ_NEW_QSTR(MP_QSTR_ViperTypeError), (mp_obj_t)&mp_type_ViperTypeError },
708+
#endif
706709
{ MP_OBJ_NEW_QSTR(MP_QSTR_ZeroDivisionError), (mp_obj_t)&mp_type_ZeroDivisionError },
707710
// Somehow CPython managed to have OverflowError not inherit from ValueError ;-/
708711
// TODO: For MICROPY_CPYTHON_COMPAT==0 use ValueError to avoid exc proliferation

py/obj.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,7 @@ extern const mp_obj_type_t mp_type_SystemExit;
434434
extern const mp_obj_type_t mp_type_TypeError;
435435
extern const mp_obj_type_t mp_type_UnicodeError;
436436
extern const mp_obj_type_t mp_type_ValueError;
437+
extern const mp_obj_type_t mp_type_ViperTypeError;
437438
extern const mp_obj_type_t mp_type_ZeroDivisionError;
438439

439440
// Constant objects, globally accessible

py/objexcept.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,10 @@ MP_DEFINE_EXCEPTION(Exception, BaseException)
252252
*/
253253
//MP_DEFINE_EXCEPTION(SystemError, Exception)
254254
MP_DEFINE_EXCEPTION(TypeError, Exception)
255+
#if MICROPY_EMIT_NATIVE
256+
MP_DEFINE_EXCEPTION_BASE(TypeError)
257+
MP_DEFINE_EXCEPTION(ViperTypeError, TypeError)
258+
#endif
255259
MP_DEFINE_EXCEPTION(ValueError, Exception)
256260
#if MICROPY_PY_BUILTINS_STR_UNICODE
257261
MP_DEFINE_EXCEPTION_BASE(ValueError)

py/qstrdefs.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,9 @@ Q(SystemExit)
144144
Q(TypeError)
145145
Q(UnboundLocalError)
146146
Q(ValueError)
147+
#if MICROPY_EMIT_NATIVE
148+
Q(ViperTypeError)
149+
#endif
147150
Q(ZeroDivisionError)
148151
#if MICROPY_PY_BUILTINS_STR_UNICODE
149152
Q(UnicodeError)

0 commit comments

Comments
 (0)