Re: [PATCH 1/1] scm_set_source_properties_x: optimize if only name, line, and/or col
On 1/17/21 2:24 PM, Rob Browning wrote:
* libguile/srcprop.c (scm_set_source_properties_x): When only a subset
of file, line, and column fields are specified, store the data as a
srcprops object (tc16_srcprops) rather than an alist.
---
Proposed for at least 3.x. If we want some additional tests, then
one option might be a public or private predicate to test for a
srcprops object (at least for scheme level testing).
Does this mean I can swap out this code for something better now? I've
disliked this code for years.
static SCM
ag_scm_c_eval_string_from_file_line(
char const * pzExpr, char const * pzFile, int line)
{
SCM port = scm_open_input_string(AG_SCM_FROM_STR(pzExpr));
if (OPT_VALUE_TRACE >= TRACE_EVERYTHING)
fprintf(trace_fp, TRACE_EVAL_STRING, pzFile, line, pzExpr);
{
static SCM file = SCM_UNDEFINED;
static char * pzOldFile = NULL;
if ((pzOldFile == NULL) || (strcmp(pzOldFile, pzFile) != 0)) {
if (pzOldFile != NULL)
AGFREE(pzOldFile);
AGDUPSTR(pzOldFile, pzFile, "scheme source");
file = AG_SCM_FROM_STR(pzFile);
}
{
SCM ln = scm_from_int(line);
scm_set_port_filename_x(port, file);
scm_set_port_line_x(port, ln);
scm_set_port_column_x(port, SCM_INUM0);
}
}
{
SCM ans = SCM_UNSPECIFIED;
/* Read expressions from that port; ignore the values. */
for (;;) {
SCM form = scm_read(port);
if (SCM_EOF_OBJECT_P(form))
break;
ans = scm_primitive_eval_x(form);
}
return ans;
}
}
[PATCH 1/1] scm_set_source_properties_x: optimize if only name, line, and/or col
* libguile/srcprop.c (scm_set_source_properties_x): When only a subset
of file, line, and column fields are specified, store the data as a
srcprops object (tc16_srcprops) rather than an alist.
---
Proposed for at least 3.x. If we want some additional tests, then
one option might be a public or private predicate to test for a
srcprops object (at least for scheme level testing).
libguile/srcprop.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/libguile/srcprop.c b/libguile/srcprop.c
index 0ebbfc301..fea68e031 100644
--- a/libguile/srcprop.c
+++ b/libguile/srcprop.c
@@ -168,8 +168,6 @@ SCM_DEFINE (scm_source_properties, "source-properties", 1,
0, 0,
#define SCM_VALIDATE_NIM(pos, scm) \
SCM_MAKE_VALIDATE_MSG (pos, scm, NIMP, "non-immediate")
-/* Perhaps this procedure should look through an alist
- and try to make a srcprops-object...? */
SCM_DEFINE (scm_set_source_properties_x, "set-source-properties!", 2, 0, 0,
(SCM obj, SCM alist),
"Install the association list @var{alist} as the source property\n"
@@ -177,9 +175,17 @@ SCM_DEFINE (scm_set_source_properties_x,
"set-source-properties!", 2, 0, 0,
#define FUNC_NAME s_scm_set_source_properties_x
{
SCM_VALIDATE_NIM (1, obj);
-
- scm_weak_table_putq_x (scm_source_whash, obj, alist);
-
+ SCM fname = scm_assq_ref (alist, scm_sym_filename);
+ SCM col = scm_assq_ref (alist, scm_sym_column);
+ SCM line = scm_assq_ref (alist, scm_sym_line);
+ const int n = ((fname == SCM_BOOL_F) ? 0 : 1)
++ ((col == SCM_BOOL_F) ? 0 : 1)
++ ((line == SCM_BOOL_F) ? 0 : 1);
+
+ if (scm_ilength (alist) == n)
+scm_i_set_source_properties_x (obj, line, col, fname);
+ else
+scm_weak_table_putq_x (scm_source_whash, obj, alist);
return alist;
}
#undef FUNC_NAME
--
2.29.2
Re: Guile 1.8.9 release
On Sun, Jan 17, 2021 at 12:10 AM Thien-Thi Nguyen wrote: > > > () Han-Wen Nienhuys > () Tue, 12 Jan 2021 09:20:55 +0100 > >your timing is fortuitous. I just spent the christmas >holidays delving into GUILE 1.8's heap expansion strategy and >found and fixed a bug with it. > > > https://github.com/hanwen/guile/commit/9b32504780e0b604196be866b8c36079891e3cd6 > >How does code review for proposed patches work these days? > > I don't know. I think what you did is fine. I invite experts > to review and comment. They will surely be quicker than me (i > will require a week or two just to read/understand the patch). > >I'd be happy to polish it up (ie. add a proper test) for >inclusion in GUILE 1.8.9, > > Sounds good. I think the general approach for 1.8.x releases > will be bugfixes and documentation changes primarily, so your > change would be most welcome (once i wrap my head around it). Thanks. It turns out my previous fix introduced ABI breakage, so I reworked it to not change function signatures or struct sizes. It's also split up in more parts, so it becomes easier to understand. Please see here: https://github.com/hanwen/guile/commit/8fbe3222cac4b4e9b39a6a3570ac43f160faa516 -- Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
