Hello! "Neil Jerram" <[EMAIL PROTECTED]> writes:
> 2008/9/15 Ludovic Courtès <[EMAIL PROTECTED]>: >> >> "Neil Jerram" <[EMAIL PROTECTED]> writes: >> >>> Now, as it happens, the code doesn't actually implement what the >>> manual says, and in fact scm_array_handle_release() is currently a >>> no-op! But I believe the manual's intention is sensible, so it think >>> I think we should commit this patch as-is for now, and raise a bug to >>> track the fact that the array/handle API isn't fully implemented. >>> >>> What do you think? >> >> I'd prefer fixing the manual rather than `scm_array_get_handle ()', >> because (1) setting up a dynwind is "costly" (involves some consing), > > Which implies that we should avoid it if not needed, which to me > implies that it makes sense to have it _inside_ the scm_array* > functions, because the implementation of those functions determines > whether it is needed. (Currently it isn't needed, because the > functions don't actually perform any reservation.) If `get_handle ()' creates a dynwind that ends in `release_handle ()', then code like the following will no longer work as expected: scm_dynwind_begin (); scm_dynwind_unwind_handler (); scm array_get_handle (); ... scm_dynwind_end (); scm_array_release_handle (); Perhaps that's what is implied by the manual's wording but I find it a bit tricky. >> and (2) I don't know of any other function that does a dynwind behind >> the scenes (IOW, let's not break the "rule of least surprise"). I meant "I don't know of a function that does a `dynwind_begin' *alone*" (of course there are plenty of functions that do `dynwind_begin' + `dynwind_end'). > I think you're imagining a clear boundary here where there isn't one. > If needed, either the scm_dynwind would be inside > scm_array_get_handle, or it would be inside scm_uniform_vector_read. > Both of those are public libguile functions, so where's the > difference? The difference is that `scm_array_get_handle ()' is a low-level function. It may be used, say, in code that passes SRFI-4 vectors to C that implements "performance-critical" code. Adding consing in there wouldn't feel right. >> OTOH, it may be the case that people have been relied on the described >> behavior, in which case it would be wiser to fix `scm_array_get_handle ()' >> to conform to the manual... > > Note that scm_array_get_handle can itself throw an error, and so can > the other APIs that return a handle to the caller. That would suggest adding a `dynwind_begin'/`dynwind_end' pair, not a `dynwind_begin' alone---but that is not actually needed, since none of the callees of `scm_array_get_handle ()' can throw an error, except for `scm_wrong_type_arg_msg ()' at the end. > One more observation: we should take care when adding occurrences of > scm_dynwind_begin (0) into code, because it prevents the enclosed code > from capturing its continuation, returning, and then calling the > continuation again later. In this case, I wonder if there could be > practical soft port implementations that would want to do this. (I > thought I had done it myself when writing (gui entry-port) for > guile-gui, but in fact that only uses a continuation as an escape > mechanism (i.e. for jumping back up the stack).) On the other hand, > if we use scm_dynwind_begin (SCM_F_DYNWIND_REWINDABLE), to allow a > continuation to be called again from outside, that raises the question > of what happens if the array in question has changed since the > continuation was captured. Right, I hadn't thought about it, but as you mention, a dynwind in `uniform-vector-read!' will only affect soft port implementations. Thanks, Ludo'.
