Hello! Maxime Devos <maximede...@telenet.be> writes:
> Op 02-10-2023 om 18:13 schreef Maxim Cournoyer: >>> 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. >> Surprisingly perhaps, this works, I guess because a string is an array. >> See the 2009 commit 31ab99de563027fe2bceb60bbd712407fcaf868e which >> exploits the same feature. > > While apparently it works, that does not look like a feature to me but > rather an accident, though I don't know what hypothetical feature you > are referring to. I meant feature here as behavior. > Also, sure strings are arrays, but you don't seem > to be using that anywhere, so I'm not following. That was just an uneducated guess about how the underlying C procedure translated from the macros. You can safely forget about it. >> %load-hook is carefully crafted as to accept both one or two arguments >> (for backward compatibility). I didn't pay attention to %load-announce >> and I'm surprised it's a public API, as its sole function seems to be >> the default %load-hook value. > > The default value might be, but it's supposed to be overwritable, and > according to the old API it's a one-argument procedure, so if there is > an old application/library setting it to something accepting only a > single argument, there is your API break. %load-announce is replaced whole via something like: set! %load-hook my-proc placed early in your program. Whether my-proc is a 1 or 2 arguments procedure, it works with the proposed change (it's been tested). > To prevent future API breaks, I propose documenting turning %load-hook > into a keyword argument procedure with #:allow-other-keys, as > something like: > > (lambda* (file-name #:key (depth the-default) #:allow-other-keys) > ...) > > and in the documentation mention that more keywords may be added in > the future (and hence #:allow-other-keys). > > I think it's quite plausible that there will be more such arguments in > the future! That sounds like a good idea, helas as I understand, with the current solution, everything needs to be kept as fixed positional arguments so we can make sense of them on the C side (which accepts a list of 1 or more items, expected to be given in order). So unless you have other ideas that would also ensure backward-compatibility concern on the C side, I'd leave it as is for now. -- Thanks, Maxim