Re: [PATCH v2 3/3] libpoke: Add `format`

[Jose E. Marchesi]

> On Sat, Jun 12, 2021 at 04:17:32PM +0200, Jose E. Marchesi wrote:
>> 
>> Hi Mohammad.
>> 
>> I have one question about the `format' support:
>> 
>> > +struct pkl_ast_format
>> > +{
>> > +  struct pkl_ast_common common;
>> > +
>> > +  const char* node_name; /* "format" or "printf" */
>> > +  int nargs;
>> > +  char *prefix;
>> > +  int fmt_processed_p;
>> > +  union pkl_ast_node *fmt;
>> > +  union pkl_ast_node *types;
>> > +  union pkl_ast_node *args;
>> > +};
>> 
>> As far as I can see `node_name' is only used to format error messages in
>> the analysis/transformation phases.
>> 
>> Like in:
>> 
>> >                PKL_ICE (PKL_AST_LOC (arg),
>> > -                       "couldn't promote printf argument initializer");
>> > +                       "couldn't promote %s argument initializer",
>> > +                       PKL_AST_FORMAT_NODE_NAME (format));
>> 
>> And in:
>> 
>> > +          PKL_ERROR (PKL_AST_LOC (format),
>> > +                     "not enough arguments in %s",
>> > +                     PKL_AST_FORMAT_NODE_NAME (format));
>> 
>> What about reformatting the error messages so we don't need this
>> attribute?  I am thinking on diagnostic messages like: "couldn't promote
>> format argument initializer" and "not enough arguments for format
>> string".
>> 
>> WDYT?
>> 
>
> I did it that way first, but I changed my mind to make things more
> explicit. 
> But I 100% agree with removing `node_name`.

The patch is OK for master, provided that.
Thank you for working on this!  It is very cool :)