[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 4/4] load: Display modules depth in output when using %loa
From: |
Maxime Devos |
Subject: |
Re: [PATCH v4 4/4] load: Display modules depth in output when using %load-verbosely. |
Date: |
Thu, 28 Sep 2023 16:23:58 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 |
Something I didn't notice previously:
Op 25-09-2023 om 16:29 schreef Maxim Cournoyer:
+ if (scm_is_string (args)) {
+ /* C code written for 3.9 and earlier expects this function to
+ take a single argument (the file name). */
+ filename = args;
+ depth = scm_from_int(0);
+ }
IIUC, args is always a list and never a string. However, it might be a
list of one element (being a string) or two elements.
>+ else {
>+ /* Starting from 3.10, this function takes 1 required and 1 >optional
>+ arguments. */
>+ long len;
>+
>+ SCM_VALIDATE_LIST_COPYLEN (SCM_ARG1, args, len);
>+ if (len < 1 || len > 2)
>+ scm_error_num_args_subr (FUNC_NAME);
>+
>+ filename = SCM_CAR (args);
>+ SCM_VALIDATE_STRING (SCM_ARG1, filename);
>+
>+ depth = len > 1 ? SCM_CADR (args) : scm_from_int(0);
While I suppose this would work, you are using the rest argument to
manually implement optional elements. I think it would be quite a bit
simpler to just use the optional elements instead. (I.e., set opt=1
instead of rst=1 and keep req=1 in SCM_DEFINE).
Adding an extra argument is, however, a C API and ABI break, so I'd
rename it to scm_primitive_load2 and let scm_primitive_load be a small
wrapper for scm_primitive_load2.
... now I think about it and re-read the C comments, maybe the (lack of)
C API/ABI break is why you are using rest arguments here? I think a new
function is cleaner and less risky though -- I don't think you are
supposed to do (primitive-load (list "a" 0)) in Scheme yet this code
allows that.
Likely the same applies to s_scm_primitive_load_path.
* doc/guile-api.alist (%load-announce, %load-hook): Add DEPTH argument.
Technically backwards-incompatible but I don't know any actual users of
%load-hook etc. outside Guile itself.
Best regards,
Maxime Devos
OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature