New Arguments and Arg/ArgWithDefault AST representation - #59

Nit: This inline in arguments... is a lot to parse. Maybe assign the result to a temporary variable. This has the advantage that it gives the "thing" a name that guides readers.

My intention was that we would use ArgWithDefault in cases where the argument has a default. I think it's a bit odd that we now have ArgWithDefault that has no default.

You are right. ArgWithDefault is only created for arg with potential default value, in common functions.

This type UntypedParameter is used for lambda expression, which disallow type hints. default is always None for lambda. If we want to give Arg for this types, we have to create Arguments variant like LambdaArguments.

Before this change, the default field of old Arguments was always empty Vec for lambda expression. I think the current change matches to it. (though we don't have to)

So do I understand this correctly that the ArgWithDefault is only a temporal data structure and we convert this into an Arg inside the lambda handling?

No, this is always ArgWithDefault.

I had confusion for the upper comment. It is always not annotated, but it can have a default.

default is possibly filled up later when it exists.

A lambda with default value currently looks like

echo "lambda a=10: a" | python -m ast
Module(
   body=[
      Expr(
         value=Lambda(
            args=arguments(
               posonlyargs=[],
               args=[
                  arg(arg='a')],
               kwonlyargs=[],
               kw_defaults=[],
               defaults=[
                  Constant(value=10)]),
            body=Name(id='a', ctx=Load())))],
   type_ignores=[])

it will look like this with this patch

Module(
   body=[
      Expr(
         value=Lambda(
            args=arguments(
               posonlyargs=[],
               args=[
                  arg_with_default(arg='a', default=Constant(value=10))],
               kwonlyargs=[],
            body=Name(id='a', ctx=Load())))],
   type_ignores=[])

When it doesn't have a default, it is arg_with_default with default=None

Module(
   body=[
      Expr(
         value=Lambda(
            args=arguments(
               posonlyargs=[],
               args=[
                  arg_with_default(arg='a')],
               kwonlyargs=[],
            body=Name(id='a', ctx=Load())))],
   type_ignores=[])

args is a sequence type. it is not possible to give it a mix of arg and arg_with_default elements without introducing a new enum type.

I think I would then just go with Arg rather than naming it ArgWithDefault.

How do you distinguish it from arg without default? (Which is currently Arg)

I guess I still don't understand how the final AST structure looks. What I understood from your last comment is that we use ArgWithDefault in all places where we used to use Arg. Is this correct?

That's here https://github.com/RustPython/Parser/pull/59/files#diff-9f23fa4f43702fa8ce10a83956426c5dd647568d6d60cd59e3dff7f78b493981R3000-R3008

ArgWithDefault is used for args, posonlyargs and kwonlyargs. Arg is used for vararg and kwarg. Becasue this is illegal in Python.

def f(*args=[something]):
    pass

You are right. ArgWithDefault is only created for arg with potential default value, in common functions.

This type UntypedParameter is used for lambda expression, which disallow type hints. default is always None for lambda. If we want to give Arg for this types, we have to create Arguments variant like LambdaArguments.

Before this change, the default field of old Arguments was always empty Vec for lambda expression. I think the current change matches to it. (though we don't have to)

@MichaReiser Is this descriptive enough?

It's also more human friendly (and implementing pre-order traversal becomes easier too)

Thank you! I added that too.