Dedicated AST nodes for patterns?

[gvanrossum]

Nick released a new version of PEP 642 and suggested that it would benefit people looking at the AST or the bytecode compiler if AST nodes for patterns didn't piggy-back on expression nodes. @brandtbucher has some misgivings about this idea, writing:

I've been thinking about this a bit too. I am open to it, but I'm not really convinced that there is much of a benefit apart from:

• removing some always-unreachable cases in anything that traverses ASTs (such as the compiler)
• relieving some AST clients of keeping track of whether or not they are in a pattern
• cleaning up the List/Tuple redundancy for sequence matches
• making wildcards their own node

Of course, it comes at the cost of adding quite a few new node types. Roughly:

• pattern
• MatchClass
• MatchMapping
• MatchSequence
• MatchConstant (also probably MatchUnaryOp and MatchBinOp for negative/complex literals, unless we want to move that step from the AST optimizer to the parser)
• MatchName
• MatchStarredName
• MatchWildcard
• MatchAttribute

It's almost reinventing the expression nodes entirely. That won't make the "patterns are a confusing DSL" folks very happy... ;)

One other thing that I really like about the current design is that some clients of the AST can work just fine by adding support for only a few new nodes. For example, ast.unparse requires very little modification to work correctly with PEP 634. If we change the nodes, unparse would have to add a bunch of new handlers that are basically aliases to existing methods.

If you think it's a good idea, though, I'll just go ahead and do it. It could be a nice chance to show that we're actually listening and are willing to learn from what Nick has presented. It just seems like a pretty big user-facing change for questionable benefit.

I'm not sure about what we should do here, but I think that for someone looking at an AST it could be helpful to see at a glance whether a particular node is a pattern or an expression (note that we have other ways to distinguish load vs. store in the area of overlap between expressions and assignments, 'context'; I don't think it would be a good idea to extend that mechanism).

[Tobias-Kohn]

I concur: it is certainly a trade-off that could go either way and I have no strong feelings for or against either variant. Of course, Nick is using some dramatic rhetoric to make his point, quite overblowing the importance of this.

Without doubt, it would be a cleaner design with dedicated nodes, but at the expense of a significant population growth in the realm of AST nodes. While I like a clean design as much as the next man, I am not too sure about having to deal with a bunch of additional nodes—although it would probably be 'honest'. As you say, reusing the 'context' field does not seem like a good idea and would IMHO lead to a more brittle and complex design.

At the end of the day, it might be a good idea to have dedicated nodes because of the resulting cleaner design that provides a better abstraction from the concrete syntax used. If you squint a little, you might even put that into the category of "explicit is better than implicit" in that the nodes along with their handlers receive more descriptive names 😉.

[gvanrossum]

(What about the next woman? :-)

[brandtbucher]

Can we agree to defer this, then? It can easily be changed in a later alpha (I'm even comfortable with changing it in an early beta if Pablo is). If somebody makes noise about an actual use case where these new nodes are a quality-of-life improvement (mypy?), then that's probably enough to convince me. The only client I've converted so far is ast.unparse, which is much simpler with the current design.

I'm really not enthusiastic about creating, documenting, supporting, and testing a dozen new stdlib classes when our collective opinion is just "meh".

[Tobias-Kohn]

Yes, I am absolutely on board with the idea of deferring it. It seems to be mostly an implementation detail that is important rather for "meta-tools". Frankly, I share your feeling of not being too excited about creating all these new classes if there is not a real need for it.

[gvanrossum]

Sounds good to me. I think mypy will survive; it has its own AST structure that is derived from ast.AST by a transformation, and if it is necessary to use different nodes to represent e.g. a class pattern than a function call (which is likely) that can be done quite cleanly in the transformation.