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

[Mohammad-Reza Nabipoor]

Hi, Jose.

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`.