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

[Mohammad-Reza Nabipoor]

Hi, Jose.

On Thu, May 27, 2021 at 08:29:40PM +0200, Jose E. Marchesi wrote:
> 
> Hi Mohammad.
> 
> Before reviewing this patch in detail I have got a general question for
> you.
> 
> Have you considered to use a single AST node type for both PRINT and
> FORMAT?  In that case, what led you to not use that approach?
> 
> The same applies to the corresponding anal, trans, etc handlers.
> 

My initial plan was
  - copy the PRINT AST node and rename it to FORMAT
  - Implement `format`
  - Re-write `print` using `format`: `print (format (...))`
  - Simplify the PRINT AST node to just a single pvm_val of type string
  - Re-write codegen for PRINT to be able to print "fat strings"

But after implementing the formater for array, I realized that printing
a large array is much more efficient than formating a large array and
then printing that big string.

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

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?


Regards,
Mohammad-Reza