[m-rev.] for review: better error messages for string.format

Julien Fischer jfischer at opturion.com
Sat Nov 15 16:19:32 AEDT 2014


On Thu, 13 Nov 2014, Zoltan Somogyi wrote:

> The motivation for the change should be apparent from
> reading the diffs for the expected output files.
>
> I am interested in comments on the XXX about whether
> we should generate an error if a format string specifies
> a useless precision for a %c conversion.

As opposed to simply ignoring it?  I don't have a particularly strong
opinion one way or the other.  (In principle, making it an error might
now might "break" existing code, but I strongly doubt the situation
arises that much in practice.)

What is of concern here is just how incomplete the documentation for
string.format is (I looked it up to see what we currently said about the
%c specifier).  The possiblity of it throwing an exception when an
invalid format string is passed to it isn't even mentioned!

> I believe the compiler won't be able to use exactly the same code
> for parsing the format string as string.format itself, because
> the representation of the list of polytypes is necessarily different.
> For string.format, all values are manifest; for the compiler,
> some are manifest, and some come from variables. While
> I can see how one piece of polymorphic code can handle this
> for values to be printed, I don't see any good enough way
> to handle this for field widths and precisions, because while
> most widths and precisions are manifest in the format string
> itself, some come from the value list itself, when the format
> string contains stuff like %*d. Besides the fact that any code
> generic enough to handle this correctly for the compiler would
> be too slow for interpreting format strings at runtime, I think
> it would also generate less-than-optimally-useful error messages.
>
> I am therefore thinking that I will move the format string
> parsing code to either a new module, or two new modules
> (one for runtime use, one for compiler use). Any objections?

Not from me.

...

> Generate better error messages for string.format.
> 
> library/string.format.m:
>     We used to parse format strings, and pair up format specifiers with
>     the list of input poly_types, using semidet code, throwing
>     exceptions with generic messages if the semidet code failed.
>     This diff switches to using det code that returns a list of
>     situation-specific error messages. Since errors after the first
>     may be caused by the first error, we now throw exceptions whose
>     message is a detailed description of the first error, but that
>     decision is easily changeable.
>
>     Simplify some existing code.
> 
> tests/general/string_format_test_{2,3}.{exp,exp2,exp3,...}:
> tests/invalid/string_format_bad.err_{exp,exp2,exp3,...}:
> tests/invalid/string_format_unknown.err_{exp,exp2,exp3,...}:
>     Update the expected error messages.

The diff appears fine.

Cheers,
Julien.



More information about the reviews mailing list