Re: User-defined record types, v2

[Eli Zaretskii]

> From: Lars Brinkhoff <address@hidden>
> Date: Sat, 18 Mar 2017 18:05:50 +0100
> 
> Add record objects with user-defined types.

Thanks.  I hope you will add documentation and some tests at some
future point.

A few minor comments below.

> +static struct Lisp_Vector *
> +allocate_record (int count)
> +{
> +  if (count >= (1 << PSEUDOVECTOR_SIZE_BITS))
> +    error ("Record too large");

I think this error should be signaled by the APIs themselves, and it
should include the max allowed number and the actual requested number.

> +DEFUN ("make-record", Fmake_record, Smake_record, 3, 3, 0,
> +       doc: /* Create a new record of type TYPE with SLOTS elements, each 
> initialized to INIT.  */)

This line is too long, I suggest to describe the arguments on separate
lines.

Also, the doc string should state the maximum allowed value of SLOTS.

> +DEFUN ("record", Frecord, Srecord, 1, MANY, 0,
> +       doc: /* Return a newly created record of type TYPE the rest of the 
> arguments as slots.

This line is too long.  It also doesn't sound right to me: perhaps
"with" is missing?

> +Any number of slots, even zero slots, are allowed.

Which is a lie, since a number that is too large will cause an error
be signaled, right?

> +  memcpy (p->contents + 1, args + 1, (nargs - 1) * sizeof *args);

Should the doc string state that a shallow copy of the arguments is
done?

> +DEFUN ("copy-record", Fcopy_record, Scopy_record, 1, 1, 0,
> +       doc: /* Shallow copy of a record.  */)

I'm not sure this doc string is detailed enough.  How about

  Return a new record that is a shallow copy of the argument RECORD.

?

> +INLINE void
> +CHECK_RECORD_TYPE (Lisp_Object x)
> +{
> +  /* CHECK_SYMBOL (x); */
> +}

???