[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 8/9] pkl: report evaluation location in E_constraint exception
|
From: |
Mohammad-Reza Nabipoor |
|
Subject: |
[PATCH 8/9] pkl: report evaluation location in E_constraint exception |
|
Date: |
Thu, 28 Dec 2023 02:19:33 +0100 |
This commit improves the UX by reporting the evaluation location
of failed constraints.
2023-12-28 Mohammad-Reza Nabipoor <mnabipoor@gnu.org>
* libpoke/pkl-insn.def (tol): New instruction.
(froml): Likewise.
(atl): Likewise.
(dropl): Likewise.
(ltrace): Likewise.
* libpoke/pvm.jitter (stack l): New stack to keep track of
source locations.
(struct pvm_exception_handler): New field to restore the
location stack after handling the exception.
(state-state-backing-c): New field `canary_locationstack'.
(canary): Initialize backing field `canary_locationstack'.
(exit): Verify that there's no left-over in the location
stack.
(tol): New instruction.
(froml): Likewise.
(atl): Likewise.
(dropl): Likewise.
(ltrace): Likewise.
(pushe): Set `location_stack_height' in exception handler.
* libpoke/pkl-gen.c (pkl_gen_pr_ass_stmt): Push location info
to location stack and drop that at the end.
(pkl_gen_ps_cons): Likewise.
(pkl_gen_pr_map): Likewise.
(pkl_gen_pr_exp_stmt): Push the location info to location
stack.
(pkl_gen_pr_struct_field): Likewise.
(pkl_gen_ps_exp_stmt): Drop the location info from location
stack.
(pkl_gen_ps_struct_ref): Likewise.
* libpoke/pkl-rt.pk (_pkl_e_constraint_msg): Add new param
for "evaluation location" of the failed constraint and
update the message.
* libpoke/pkl-gen.pks (check_struct_field_constraint):
Pass the location to `_pkl_e_constraint_msg' function.
* testsuite/poke.pickles/pcap-test.pk (tests): Fix the
match for exception message.
---
ChangeLog | 39 ++++++++++
libpoke/pkl-gen.c | 77 +++++++++++++++++++
libpoke/pkl-gen.pks | 1 +
libpoke/pkl-insn.def | 5 ++
libpoke/pkl-rt.pk | 9 ++-
libpoke/pvm.jitter | 113 ++++++++++++++++++++++++++++
testsuite/poke.pickles/pcap-test.pk | 3 +-
7 files changed, 243 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 68b944c0..020cd02d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,42 @@
+2023-12-28 Mohammad-Reza Nabipoor <mnabipoor@gnu.org>
+
+ * libpoke/pkl-insn.def (tol): New instruction.
+ (froml): Likewise.
+ (atl): Likewise.
+ (dropl): Likewise.
+ (ltrace): Likewise.
+ * libpoke/pvm.jitter (stack l): New stack to keep track of
+ source locations.
+ (struct pvm_exception_handler): New field to restore the
+ location stack after handling the exception.
+ (state-state-backing-c): New field `canary_locationstack'.
+ (canary): Initialize backing field `canary_locationstack'.
+ (exit): Verify that there's no left-over in the location
+ stack.
+ (tol): New instruction.
+ (froml): Likewise.
+ (atl): Likewise.
+ (dropl): Likewise.
+ (ltrace): Likewise.
+ (pushe): Set `location_stack_height' in exception handler.
+ * libpoke/pkl-gen.c (pkl_gen_pr_ass_stmt): Push location info
+ to location stack and drop that at the end.
+ (pkl_gen_ps_cons): Likewise.
+ (pkl_gen_pr_map): Likewise.
+ (pkl_gen_pr_exp_stmt): Push the location info to location
+ stack.
+ (pkl_gen_pr_struct_field): Likewise.
+ (pkl_gen_ps_exp_stmt): Drop the location info from location
+ stack.
+ (pkl_gen_ps_struct_ref): Likewise.
+ * libpoke/pkl-rt.pk (_pkl_e_constraint_msg): Add new param
+ for "evaluation location" of the failed constraint and
+ update the message.
+ * libpoke/pkl-gen.pks (check_struct_field_constraint):
+ Pass the location to `_pkl_e_constraint_msg' function.
+ * testsuite/poke.pickles/pcap-test.pk (tests): Fix the
+ match for exception message.
+
2023-12-28 Mohammad-Reza Nabipoor <mnabipoor@gnu.org>
* libpoke/pkl-ast.c (pkl_ast_handle_bconc_ass_stmt_1):
diff --git a/libpoke/pkl-gen.c b/libpoke/pkl-gen.c
index b94306d8..c350d5b9 100644
--- a/libpoke/pkl-gen.c
+++ b/libpoke/pkl-gen.c
@@ -850,6 +850,14 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_ass_stmt)
int assigning_computed_field_p = 0;
int assigning_to_indirection_p = 0;
const char *computed_field_name = NULL;
+ char *loc = pkl_ast_format_loc (PKL_GEN_PAYLOAD->filename,
+ PKL_AST_LOC (PKL_PASS_NODE));
+
+ /* Push the location to location stack. */
+ assert (loc != NULL);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_PUSH, pvm_make_string (loc));
+ free (loc);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_TOL);
if (exp)
PKL_PASS_SUBPASS (exp);
@@ -1254,6 +1262,9 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_ass_stmt)
|| PKL_AST_CODE (lvalue) == PKL_AST_STRUCT_REF)
pkl_asm_label (PKL_GEN_ASM, done);
+ /* Drop the location from location stack. */
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_DROPL);
+
PKL_PASS_BREAK;
#undef LMAP
@@ -1493,6 +1504,24 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_ps_return_stmt)
}
PKL_PHASE_END_HANDLER
+/*
+ * EXP_STMT
+ * | EXP
+ */
+
+PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_exp_stmt)
+{
+ char *loc = pkl_ast_format_loc (PKL_GEN_PAYLOAD->filename,
+ PKL_AST_LOC (PKL_PASS_NODE));
+
+ /* Push the location to location stack. */
+ assert (loc != NULL);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_PUSH, pvm_make_string (loc));
+ free (loc);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_TOL);
+}
+PKL_PHASE_END_HANDLER
+
/*
* | EXP
* EXP_STMT
@@ -1507,6 +1536,9 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_ps_exp_stmt)
&& PKL_AST_CODE (PKL_PASS_PARENT) == PKL_AST_PROGRAM)
|| PKL_GEN_PAYLOAD->in_file_p)
pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_DROP);
+
+ /* Drop the location from location stack. */
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_DROPL);
}
PKL_PHASE_END_HANDLER
@@ -2642,6 +2674,14 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_ps_cons)
pkl_ast_node cons = PKL_PASS_NODE;
int cons_kind = PKL_AST_CONS_KIND (cons);
pkl_ast_node cons_type = PKL_AST_CONS_TYPE (cons);
+ char *loc = pkl_ast_format_loc (PKL_GEN_PAYLOAD->filename,
+ PKL_AST_LOC (cons));
+
+ /* Push the location to location stack. */
+ assert (loc != NULL);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_PUSH, pvm_make_string (loc));
+ free (loc);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_TOL);
switch (cons_kind)
{
@@ -2661,6 +2701,9 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_ps_cons)
default:
PK_UNREACHABLE ();
}
+
+ /* Drop the location from location stack. */
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_DROPL);
}
PKL_PHASE_END_HANDLER
@@ -2677,6 +2720,13 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_map)
pkl_ast_node map_offset = PKL_AST_MAP_OFFSET (map);
pkl_ast_node map_ios = PKL_AST_MAP_IOS (map);
pkl_ast_node map_type = PKL_AST_MAP_TYPE (map);
+ char *loc = pkl_ast_format_loc (PKL_GEN_PAYLOAD->filename, PKL_AST_LOC
(map));
+
+ /* Push the location to location stack. */
+ assert (loc != NULL);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_PUSH, pvm_make_string (loc));
+ free (loc);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_TOL);
/* Traverse the map type in normal context. */
PKL_GEN_PUSH_CONTEXT;
@@ -2745,6 +2795,9 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_map)
PKL_GEN_POP_CONTEXT;
}
+ /* Drop the location from location stack. */
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_DROPL);
+
PKL_PASS_BREAK;
}
PKL_PHASE_END_HANDLER
@@ -3014,6 +3067,25 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_struct_field)
}
PKL_PHASE_END_HANDLER
+/*
+ * STRUCT_REF
+ * | STRUCT
+ * | IDENTIFIER
+ */
+
+PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_struct_ref)
+{
+ char *loc = pkl_ast_format_loc (PKL_GEN_PAYLOAD->filename,
+ PKL_AST_LOC (PKL_PASS_NODE));
+
+ /* Push the location to location stack. */
+ assert (loc != NULL);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_PUSH, pvm_make_string (loc));
+ free (loc);
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_TOL);
+}
+PKL_PHASE_END_HANDLER
+
/*
* | STRUCT
* | IDENTIFIER
@@ -3182,6 +3254,9 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_ps_struct_ref)
break;
}
}
+
+ /* Drop the location from location stack. */
+ pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_DROPL);
}
PKL_PHASE_END_HANDLER
@@ -5086,6 +5161,7 @@ struct pkl_phase pkl_phase_gen =
PKL_PHASE_PR_HANDLER (PKL_AST_LOOP_STMT, pkl_gen_pr_loop_stmt),
PKL_PHASE_PR_HANDLER (PKL_AST_RETURN_STMT, pkl_gen_pr_return_stmt),
PKL_PHASE_PS_HANDLER (PKL_AST_RETURN_STMT, pkl_gen_ps_return_stmt),
+ PKL_PHASE_PR_HANDLER (PKL_AST_EXP_STMT, pkl_gen_pr_exp_stmt),
PKL_PHASE_PS_HANDLER (PKL_AST_EXP_STMT, pkl_gen_ps_exp_stmt),
PKL_PHASE_PR_HANDLER (PKL_AST_FORMAT, pkl_gen_pr_format),
PKL_PHASE_PR_HANDLER (PKL_AST_PRINT_STMT, pkl_gen_pr_print_stmt),
@@ -5118,6 +5194,7 @@ struct pkl_phase pkl_phase_gen =
PKL_PHASE_PR_HANDLER (PKL_AST_STRUCT, pkl_gen_pr_struct),
PKL_PHASE_PS_HANDLER (PKL_AST_STRUCT, pkl_gen_ps_struct),
PKL_PHASE_PR_HANDLER (PKL_AST_STRUCT_FIELD, pkl_gen_pr_struct_field),
+ PKL_PHASE_PR_HANDLER (PKL_AST_STRUCT_REF, pkl_gen_pr_struct_ref),
PKL_PHASE_PS_HANDLER (PKL_AST_STRUCT_REF, pkl_gen_ps_struct_ref),
PKL_PHASE_PR_HANDLER (PKL_AST_STRUCT_TYPE_FIELD,
pkl_gen_pr_struct_type_field),
PKL_PHASE_PR_HANDLER (PKL_AST_ASM_STMT, pkl_gen_pr_asm_stmt),
diff --git a/libpoke/pkl-gen.pks b/libpoke/pkl-gen.pks
index 30286455..7e3e6c01 100644
--- a/libpoke/pkl-gen.pks
+++ b/libpoke/pkl-gen.pks
@@ -804,6 +804,7 @@
.let #code_loc = \
pvm_make_string (PKL_AST_STRUCT_TYPE_FIELD_CONSTRAINT_LOC (@field))
push #code_loc
+ atl
.call _pkl_e_constraint_msg
sset
raise
diff --git a/libpoke/pkl-insn.def b/libpoke/pkl-insn.def
index f1f15f59..d665ae51 100644
--- a/libpoke/pkl-insn.def
+++ b/libpoke/pkl-insn.def
@@ -78,6 +78,10 @@ PKL_DEF_INSN(PKL_INSN_TOR,"","tor")
PKL_DEF_INSN(PKL_INSN_FROMR,"","fromr")
PKL_DEF_INSN(PKL_INSN_ATR,"","atr")
PKL_DEF_INSN(PKL_INSN_QUAKE,"","quake")
+PKL_DEF_INSN(PKL_INSN_TOL,"","tol")
+PKL_DEF_INSN(PKL_INSN_FROML,"","froml")
+PKL_DEF_INSN(PKL_INSN_ATL,"","atl")
+PKL_DEF_INSN(PKL_INSN_DROPL,"","dropl")
PKL_DEF_INSN(PKL_INSN_REVN,"n","revn")
@@ -478,6 +482,7 @@ PKL_DEF_INSN(PKL_INSN_NOTE,"v","note")
PKL_DEF_INSN(PKL_INSN_SIZ,"","siz")
PKL_DEF_INSN(PKL_INSN_STRACE,"n","strace")
PKL_DEF_INSN(PKL_INSN_RTRACE,"n","rtrace")
+PKL_DEF_INSN(PKL_INSN_LTRACE,"n","ltrace")
PKL_DEF_INSN(PKL_INSN_DISAS,"","disas")
PKL_DEF_INSN(PKL_INSN_RAND,"","rand")
PKL_DEF_INSN(PKL_INSN_TIME,"","time")
diff --git a/libpoke/pkl-rt.pk b/libpoke/pkl-rt.pk
index 5645b18e..62333bf6 100644
--- a/libpoke/pkl-rt.pk
+++ b/libpoke/pkl-rt.pk
@@ -111,7 +111,8 @@ immutable var E_stack
Also, CODE may be empty. */
immutable fun _pkl_e_constraint_msg = (string fname, string code,
- string location) string:
+ string def_location,
+ string use_location) string:
{
var msg = "";
@@ -122,9 +123,11 @@ immutable fun _pkl_e_constraint_msg = (string fname,
string code,
msg += (c == '\n') ? "\n " : c as string;
}
- msg += "\ndefined at " + location;
if (fname != "")
- msg += " in field " + fname;
+ msg += "\nin field " + fname + " ";
+ else
+ msg += "\n";
+ msg += "evaluated at " + use_location + " (defined at " + def_location + ")";
return msg;
}
diff --git a/libpoke/pvm.jitter b/libpoke/pvm.jitter
index cdf78947..dd5dc2ec 100644
--- a/libpoke/pvm.jitter
+++ b/libpoke/pvm.jitter
@@ -51,6 +51,15 @@ stack x
guard-overflow
end
+stack l
+ long-name "locationstack"
+ c-element-type "pvm_val"
+ non-tos-optimized
+ element-no 65536
+ guard-underflow
+ guard-overflow
+end
+
## Register classes.
@@ -203,6 +212,7 @@ early-header-c
int exception;
jitter_stack_height main_stack_height;
jitter_stack_height return_stack_height;
+ jitter_stack_height location_stack_height;
pvm_program_point code;
pvm_env env;
};
@@ -266,6 +276,7 @@ late-header-c
{ \
JITTER_SET_HEIGHT_STACK (ehandler.main_stack_height); \
JITTER_SET_HEIGHT_RETURNSTACK (ehandler.return_stack_height); \
+ JITTER_SET_HEIGHT_LOCATIONSTACK (ehandler.location_stack_height);\
\
JITTER_PUSH_STACK ((EXCEPTION)); \
\
@@ -949,6 +960,7 @@ state-struct-backing-c
jitter_stack_height canary_stack;
jitter_stack_height canary_returnstack;
jitter_stack_height canary_exceptionstack;
+ jitter_stack_height canary_locationstack;
pvm vm;
ios_context ios_ctx;
end
@@ -977,6 +989,7 @@ state-initialization-c
jitter_state_backing->canary_stack = NULL;
jitter_state_backing->canary_returnstack = NULL;
jitter_state_backing->canary_exceptionstack = NULL;
+ jitter_state_backing->canary_locationstack = NULL;
jitter_state_backing->exit_code = PVM_EXIT_OK;
jitter_state_backing->result_value = PVM_NULL;
jitter_state_backing->ios_ctx = NULL;
@@ -1023,6 +1036,8 @@ instruction canary ()
JITTER_HEIGHT_RETURNSTACK ();
PVM_STATE_BACKING_FIELD (canary_exceptionstack) =
JITTER_HEIGHT_EXCEPTIONSTACK ();
+ PVM_STATE_BACKING_FIELD (canary_locationstack) =
+ JITTER_HEIGHT_LOCATIONSTACK ();
end
end
@@ -1059,6 +1074,9 @@ instruction exit ()
if (PVM_STATE_BACKING_FIELD (canary_exceptionstack) != NULL)
PVM_ASSERT (PVM_STATE_BACKING_FIELD (canary_exceptionstack)
== JITTER_HEIGHT_EXCEPTIONSTACK ());
+ if (PVM_STATE_BACKING_FIELD (canary_locationstack) != NULL)
+ PVM_ASSERT (PVM_STATE_BACKING_FIELD (canary_locationstack)
+ == JITTER_HEIGHT_LOCATIONSTACK ());
/* Clear pending signals. */
{
@@ -2027,6 +2045,68 @@ instruction pec ()
end
end
+
+## Location stack manipulation instructions
+
+# Instruction: tol
+#
+# Pop an element from the stack and push it in the location stack.
+#
+# Stack: ( VAL -- )
+# LocationStack: ( -- VAL )
+
+instruction tol ()
+ code
+ JITTER_PUSH_LOCATIONSTACK (JITTER_TOP_STACK ());
+ JITTER_DROP_STACK ();
+ end
+end
+
+# Instruction: froml
+#
+# Pop an element from the location stack and push it on the stack.
+#
+# Stack: ( -- VAL)
+# LocationStack: ( VAL -- )
+
+instruction froml ()
+ code
+ PVM_ASSERT (JITTER_HEIGHT_LOCATIONSTACK ()
+ > PVM_STATE_BACKING_FIELD (canary_locationstack));
+ JITTER_PUSH_STACK (JITTER_TOP_LOCATIONSTACK ());
+ JITTER_DROP_LOCATIONSTACK ();
+ end
+end
+
+# Instruction: atl
+#
+# Push a copy of the element at the top of the location stack into the
+# stack.
+#
+# Stack: ( -- VAL )
+
+instruction atl ()
+ code
+ PVM_ASSERT (JITTER_HEIGHT_LOCATIONSTACK ()
+ > PVM_STATE_BACKING_FIELD (canary_locationstack));
+ JITTER_PUSH_STACK (JITTER_TOP_LOCATIONSTACK ());
+ end
+end
+
+# Instruction: dropl
+#
+# Pop the value at the top of the location stack, and discard it.
+#
+# LocationStack: ( VAL -- )
+
+instruction dropl ()
+ code
+ PVM_ASSERT (JITTER_HEIGHT_LOCATIONSTACK ()
+ > PVM_STATE_BACKING_FIELD (canary_locationstack));
+ JITTER_DROP_LOCATIONSTACK ();
+ end
+end
+
## Printing Instructions
@@ -6977,6 +7057,7 @@ instruction pushe (?l)
JITTER_DROP_STACK ();
ehandler.main_stack_height = JITTER_HEIGHT_STACK ();
ehandler.return_stack_height = JITTER_HEIGHT_RETURNSTACK ();
+ ehandler.location_stack_height = JITTER_HEIGHT_LOCATIONSTACK ();
ehandler.code = JITTER_ARGP0;
ehandler.env = PVM_STATE_RUNTIME_FIELD (env);
@@ -7131,6 +7212,38 @@ instruction vmdisp ()
end
end
+instruction ltrace (?n)
+ non-relocatable
+ code
+ int i = 0;
+ int num_elems = (int) JITTER_ARGN0;
+ int num_elems_in_stack;
+
+ PVM_ASSERT (PVM_STATE_BACKING_FIELD (canary_locationstack) != NULL);
+
+ num_elems_in_stack
+ = (pvm_val *)JITTER_HEIGHT_LOCATIONSTACK ()
+ - (pvm_val *)PVM_STATE_BACKING_FIELD (canary_locationstack);
+ if (num_elems == 0 || num_elems > num_elems_in_stack)
+ num_elems = num_elems_in_stack;
+
+ while (i < num_elems)
+ {
+ pvm_print_val_with_params (PVM_STATE_BACKING_FIELD (vm),
+ JITTER_AT_DEPTH_LOCATIONSTACK (i),
+ 0 /* depth */,
+ PVM_PRINT_FLAT,
+ 16 /* base */,
+ 2 /* indent */,
+ 0 /* acutoff */,
+ PVM_PRINT_F_MAPS,
+ NULL /* exit_exception */);
+ pk_puts (pvm_literal_newline);
+ i++;
+ }
+ end
+end
+
## System Interaction Instructions
diff --git a/testsuite/poke.pickles/pcap-test.pk
b/testsuite/poke.pickles/pcap-test.pk
index e26dab36..94dca478 100644
--- a/testsuite/poke.pickles/pcap-test.pk
+++ b/testsuite/poke.pickles/pcap-test.pk
@@ -215,7 +215,8 @@ var tests = [
catch (Exception ex)
{
assert (ex.code == EC_constraint);
- assert (ex.msg[ex.msg'length-17:] == "PCAP_Header.magic");
+ printf ("ex:%v\n", ex);
+ assert (strstr (ex.msg, "PCAP_Header.magic") <
ex.msg'length);
return;
}
assert (0, "Unreachable reached!");
--
2.42.1
- [PATCH 7/9] pkl: add location info to synthesized assignment stmts, (continued)
- [PATCH 6/9] pkl: change `pkl_ast_format_loc' function, Mohammad-Reza Nabipoor, 2023/12/27
- [PATCH 5/9] pkl: keep track of current filename in codgen, Mohammad-Reza Nabipoor, 2023/12/27
- [PATCH 8/9] pkl: report evaluation location in E_constraint exception,
Mohammad-Reza Nabipoor <=
- [PATCH 9/9] pkl: fix location info for unary/binary/ternary exp, Mohammad-Reza Nabipoor, 2023/12/27
- Re: [PATCH 1/9] utils: fix build and distribution of `pk-jojopatch', Jose E. Marchesi, 2023/12/28