Re: [PATCH v1 02/36] Guile extension language: doc additions

[Doug Evans <xdje42 at gmail dot com>]

On Wed, Dec 25, 2013 at 11:26 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 24 Dec 2013 11:02:24 -0800
>> From: Doug Evans <xdje42@gmail.com>
>>
>> This patch has the doc additions.
>> There's still some to be written, but I think it's in good enough shape
>> to get feedback on.
>
> Thanks.
>
>> +Guile version 2.0.9 is well tested, earlier 2.0 versions are not.
>
> I'm not sure we should say this in the manual.  Would you like to keep
> track of others testing other versions and update this as they report
> to you?  I don't think so.
>
> It might be good for NEWS, though.

Done.

>> +The implementation uses Guile's @code{smob} (small object)
>                                                 ^^^^^^^^^^^^
> This should be in @dfn, as it's new terminology.  Also, an index entry
> about that would be nice.

Done.  Plus I added reference to the Guile docs for smobs.

>> +@cindex guile directory
>
> I think "guile scripts directory" is a better index entry.

Done.

>> +@item guile @r{[}@var{command}@r{]}
>> +@itemx gu @r{[}@var{command}@r{]}
>> +The @code{guile} command can be used to evaluate a Scheme expression.
>
> If it's a Scheme expression, then why do you use "command" in the
> @item? why not "scheme-expression"?

Done.

>> +See the Guile documentation for a description of this function.
>
> A cross-reference to the appropriate node in the Guile manual would be
> good there.

Done.

>> +Guile's history mechanism uses the same naming as @value{GDBN}'s,
>> +namely the user of dollar-variables (e.g., $1, $2, etc.).
>> +However, the values are independent, @code{$1} in Guile is not the
>> +same value as @code{$1} in @value{GDBN}.
>
> This is not clear enough.  I'm guessing you wanted to say that results
> of evaluations in Guile and in GDB are counted separately?  If so, I
> suggest to say just that.

Done.

>> +@value{GDBN} is not thread-safe.  If your Guile program uses multiple
>> +threads, you must be careful to only call @value{GDBN}-specific
>> +functions in the main @value{GDBN} thread.
>
> What is "the main GDB thread" here?

I think the current wording is sufficient.
I'm not sure what else to say, and Python says something similar.
Python also has an example which may help.
There's no example for Guile (yet) because the example
uses functionality not implemented yet (events).

>> +The rest of this manual assumes the @code{gdb} module has been imported
>> +without any prefix.  See the Guile documentation for @code{use-modules}
>> +for more information.
>
> Again, a cross-reference would be good here.

Done.

>> +By default, any output produced by @var{command} is sent to
>> +@value{GDBN}'s standard output.
>
> What happens if logging has been turned on?

It's handled no different than typing the command into gdb.
Do we need to say more here?

>> +@emph{Note:} @value{GDBN}'s value history is independent of Guile's.
>> +@code{$1} in @value{GDBN}'s value history is not @code{$1} from Guile's
>> +history, nor is the reverse true.
>
> See comment about this above.  Also, which history are you talking
> about here: the one from Guile evaluation or the one from GDB?

Not sure I understand, the text refers to both.
$1 when used in a Scheme expression is not the same value as $1 used
in a GDB expression.
I'm happy to reword this as desired, I'm just not sure what.

>> +@node Guile Configuration
>> +@subsubsection Guile Configuration
>
> An index entry would be nice here.

Done.

>> +@defun data-directory
>> +Return a string containing @value{GDBN}'s data directory.
>
> Should we mention that this string is in UTF-8 (I think) encoding?

Strings don't have an encoding per se, they're sequences of unicode code points.
[One needs to specify an encoding when reading/writing them to a file
or whatever of course.]

>> +@defun make-exception key args
>> +Return a @code{<gdb:exception>} object.
>> +@var{key} and @var{args} are the standard Guile parameters of an exception.
>> +See Guile documentation for more information.
>    ^^^^^^^^^^^^^^^^^^^^^^^
> Cross-reference, please.

Done.

>> +@defun exception? object
>> +Return @code{#t} if @var{object} is a @code{<gdb:exception>} object.
>
> And #f otherwise, I guess.

Yeah.  Type predicates are pretty standard in Scheme so I left that as implicit.
I added another sentence to v2.

>> +@value{GDBN} does not memoize @code{<gdb:value>} objects.
>
> Why not?

I wasn't sure if there would be any unintended consequences, so I've
left it for another day.
A case of "baby steps ...".
I do memoize other things that felt important enough for this pass,
e.g. symbols.

>> +Therefore @code{eq?} does not work as expected.
>> +However @code{equal?} does work.
>> +
>> +@smallexample
>> +(gdb) guile (eq? (make-value 1) (make-value 1))
>> +$1 = #f
>> +(gdb) guile (equal? (make-value 1) (make-value 1))
>> +$1 = #t
>> +@end smallexample
>
> Wouldn't this be confusing for Scheme programmers?  Is it terribly
> hard to make eq? work?

See above.

>> +@defun value? object
>> +Return @code{#t} if @var{object} is a @code{<gdb:value>} object.
>
> And #f otherwise?

Sentence added (everywhere I could find).

>> +Many Scheme values can be converted directly to a @code{<gdb:value>} via
>> +with this procedure.
>
> Either "via" or 'with", but not both.

Yikes, I messed up there alright.  Fixed.

>> +A Scheme boolean is converted to @var{type} if provided, otherwise
>
> You already described how "type" is handled, no need to repeat that
> (here and elsewhere in this part).

If a type is not provided I need to say what happens and it's
different for each kind of value.
It's not clear to me how to distinguish the two cases in prose without
having something like the text that is there now.
Suggestions?

>> +If @var{type} is not provided,
>> +a Scheme real is converted to the C @code{double} type for the
>> +current architecture.
>
> Isn't Guile built with libgmp?  If so, doesn't it support floats
> which, when converted to a double, will lose accuracy?
>
> Also, what about long double, if it is supported by the host?

Defaulting to double is reasonable, and if the user wants a different type
(C float or C long double) then they can explicitly specify it.

>> +A Scheme string is converted to a target string, using the current
>> +target encoding.
>
> What if target encoding doesn't support some of the characters in the
> string?

An exception is thrown.  I've added more text.
[It would be nice to provide some control here, for now the
conversion is strict.]

> And what does "target string" mean anyway?  A string in C and a string
> in Fortran are two different objects, no?

The current language is used.  I've added more text.

>> +If @var{value} is a Scheme bytevector and @var{type} is provided,
>> +@var{value} must be the same size, in bytes, of values of type @var{type},
>> +and the result is essentially created by using @code{memcpy}.
>
> Can type be 'double' in this case?  If so, what happens with denormals
> and such likes that are created by such a memcpy?  Wouldn't it be
> better to allow only integer types here?

There's no need to disallow constructing denormals and such,  the user
may wish to create one.
The intent here is to provide a raw interface to pass values into and
out of Scheme.

>> +If @var{value} is a Scheme bytevector and @var{type} is not provided,
>> +the result is an array of type @code{uint8} of the same length.
>
> I would suggest using 'unsigned char' instead of uint8.

I like uint8.  For example, one can create a bytevector with "#vu8(1 2
3 4)" in Scheme.

>> +A similar function @code{value-referenced-value} exists which also
>> +returns @code{<gdb:value>} objects corresonding to the values pointed to
>> +by pointer values (and additionally, values referenced by reference
>> +values).  However, the behavior of @code{value-dereference}
>> +differs from @code{value-referenced-value} by the fact that the
>> +behavior of @code{value-dereference} is identical to applying the C
>> +unary operator @code{*} on a given value.  For example, consider a
>> +reference to a pointer @code{ptrref}, declared in your C@t{++} program
>> +as
>> +
>> +@smallexample
>> +typedef int *intptr;
>> +...
>> +int val = 10;
>> +intptr ptr = &val;
>> +intptr &ptrref = ptr;
>> +@end smallexample
>
> Here and below you describe 2 related functions, and explain how they
> are different twice, each explanation is part of its @defun.  This in
> effect says the same things twice slightly differently, and is thus
> confusing.
>
> Instead, I suggest to provide a concise description of both functions,
> and then explain their differences only once.

It doesn't feel confusing to me.
Plus it's nice to have each function's docs relatively self-contained.
Can I keep it as is for now?

>> +Each element of list @var{arg-list} must be a <gdb:value> object or an object
>> +that can be converted to one.
>                ^^^^^^^^^^^^^^^^
> "converted to a value" is less ambiguous.

Done.

>> +For C-like languages, a value is a string if it is a pointer to or an
>> +array of characters or ints.
>
> "Array of ints" meaning a wchar_t string?  If so, they might not be
> ints on some platforms (e.g., Windows).

The underlying gdb code watches for wchar_t, char16_t, and char32_t.
ref: c-lang.c:classify_type.

I've reworded the text to mention those types.

>> +The optional @var{errors} argument is either @code{"strict"}
>> +or @code{"replace"}.  A value of @code{"strict"} corresponds to
>> +Guile's @code{SCM_FAILED_CONVERSION_ERROR} and a value of @code{"replace"}
>> +corresponds to Guile's @code{SCM_FAILED_CONVERSION_QUESTION_MARK}.
>
> Suggest a cross-reference to Guile documentation here.

Done.

>> +If the optional @var{length} argument is given, the string will be
>> +fetched and encoded to the length of characters specified.  If
>> +the @var{length} argument is not provided, the string will be fetched
>> +and encoded until a null of appropriate width is found.
>
> Isn't this null termination description skewed towards C-like
> languages?  Aren't there languages where strings don't have to be
> null-terminated?

C-like languages are the most popular, plus what's described is what
the underlying gdb support provides.
If/when something more is needed it can be added.

>> +@defun value-max a b
>> +@end defun
>> +
>> +@defun value-max a b
>> +@end defun
>
> One of these should be value-min, I presume.

Fixed.

>> +@table @code
>> +@findex TYPE_CODE_PTR
>> +@item TYPE_CODE_PTR
>
> Using @ftable here would have saved you from the need to type @findex
> for each @item.

Ah.  I looked at the texinfo docs and see there's also @vtable.
Since these are variables (constants really) I've used that.

>> +@findex TYPE_CODE_INTERNAL_FUNCTION
>> +@item TYPE_CODE_INTERNAL_FUNCTION
>> +A function internal to @value{GDBN}.  This is the type used to represent
>> +convenience functions.
>
> A cross-reference to where convenience functions are described would
> be nice here.

Righto.  But that needs to wait until support for convenience
functions is implemented.

>> +@anchor{Fields of a Type in Guile}
>
> Why capitalized words?

Fixed.
@anchor{Fields of a type in Guile}

>> +@node Guile Pretty Printing API
>> +@subsubsection Guile Pretty Printing API
>
> @cindex missing here.

Done.

>> +@node Selecting Guile Pretty-Printers
>> +@subsubsection Selecting Guile Pretty-Printers
>
> @cindex.

Done.

>> +For compatibility @samp{"b"} (binary) may also be present,
>> +but we ignore it: memory ports are binary only.
>
> Which means strings read from memory cannot be decoded?

The data comes in as binary (a stream of bytes), but how it gets
decoded is up to the code doing the reading.

>> +on can be accessed.  If only @var{size} if specified, all memory in the
>> +range [0,@var{size} can be accessed.  If both are specified, all memory
>          ^^^^^^^^^^^^^
> Missing closing paren.

Fixed.

>> +@node Guile Printing Module
>> +@subsubsection Guile Printing Module
>> +@cindex (gdb printing)
>
> Is this really a useful index entry?

Thought so, but to be honest I'm not sure.
I've removed it, it can always be added later.

>> +@node Guile Types Module
>> +@subsubsection Guile Types Module
>> +@cindex (gdb types)
>
> Likewise here.

Ditto.

> Thanks again for doing this.

I think I've addressed all the points in the subsequent emails in this thread.
[if not it's just accidental oversight]

v2 coming up soon.