Re: [PATCH 11/12] libpoke: Add `format`

[Mohammad-Reza Nabipoor]

On Thu, May 27, 2021 at 10:22:08PM +0200, Jose E. Marchesi wrote:
> 
> > For sure the current patch introduces code duplication for all phases
> > except the codegen phase (it is just literal copy of the printing code
> > with s/print/format).
> >
> > I decided to postpone the unification of the two nodes, because maybe
> > there's a chance of getting ride of `print` by using lazy-strings and
> > `format`.
> 
> Well, precisely: if we unify the AST nodes now, into something that can
> accomodate both PRINT and FORMAT, then it will be just a matter of
> renaming it when it is time for PRINT to go away :)
> 
> There is a detail however: print statements are... statements, and
> therefore PKL_AST_PRINT_STMT lays between PKL_AST_FIRST_STMT and
> PKL_AST_LAST_STMT.
> 
> On the other side, `format' is an expression that returns a value, not a
> statement.
> 
> To resolve this, we could have two AST node types, still sharing the
> implementation:
> 
>    PKL_AST_FORMAT
>    - Contains all the attributes like fmt, types, etc.
> 
>    PKL_AST_PRINT_STMT
>     - Contains a PKL_AST_FORMAT.
> 
> That way, we will still need phase handlers for PKL_AST_FORMAT.  But the
> real work can then be in functions in pkl-ast.c, used by both
> PKL_AST_FORMAT and PKL_AST_PRINT_STMT.  Or macros.
> 
> WDYT?
> 


I agree.
You are right.


> >
> > The only part that requires reviewing is the codegen part.
> >
> > Should I unify the AST node in this patch, or I can do it in a subsequent
> > patch?
> 
> I would really like to avoid all that code replication, even if it is
> temporary.
> 

OK. I'll re-write this patch and will send it later.
And it'd be nice if you review the code generation part, so I can use
your feedback in the new patch.


Thanks!