Re: [PATCH v4 4/4] load: Display modules depth in output when using %load-verbosely.

[Maxime Devos]

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