bpo-39981: Default values for AST nodes

[isidentical]

https://bugs.python.org/issue39981

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[pablogsal]

Some initial comments, without commenting on the approach itself.

@serhiy-storchaka should confirm that he is happy with this approach as he was working on some related functionality.

[pablogsal]

I don't understand this function. When we set the node_type._field_defaults we could already place None or [] in there instead of this intermediary "?" and "*".

[isidentical]

I dont think so, at least for lists. We are keeping _field_defaults as the same way we keep _fields, arrays of const char pointers. And then we construct a tuple from them and intern the values. I dont think it would be practical to do this with the original values.

[pablogsal]

I don't fully understand the problem. What is wrong with having _field_defaults as a list of the defaults themselves? When constructing the _field_defaults tuple you can do this logic already and populate them with None or the empty list or whatever, no?

For instance, in the make_type function you could set the defaults in the tuple instead of interning the strings.

[isidentical]

Ah, that makes sense. Do you have an idea about what we should put if there is no field default, I think Ellipsis should work.

[pablogsal]

You don't need that if you use a dictionary instead of a tuple

[isidentical]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@pablogsal: please review the changes made to this pull request.

[isidentical]

@serhiy-storchaka It would be awesome to have your review / thoughts on this before alpha cut.

[isidentical]

Closing this in favor of #21417