v0.2 ExprWithPath

[acl33]

Addresses #808 .

This is a first stab at refactoring the "Location" logic in the python rewriter. The current design has lead to knowledge of the Location representation working its way into some places where it shouldn't, e.g. here.

edit: now second attempt - see first version in commit 025c33fd989a4bbe0ce6b94a0533775d18ad4b2f which defined PathElement = Union[str, int] so an example path ("body", 0)]

The idea here is to manipulate paths via a NamedTuple class that holds the path itself tightly together with the part of the expr that it points to, as outlined in #808; and elements of paths being function-like objects identified as path.Let_body or path.Call_args[2].

Significant questions remain:

• The Visitor class is a bit messy. It seems useful to have a Visitor-like class for ExprWithPath, and also for Expr (not with path); here these are the same class but they could be two separate classes. (I kept a single ExprVisitor class that handled both, because of the subclass ExprTransformer: if we had separate ExprVisitor and ExprWithPathVisitor, then we would need both ExprTransformer and ExprWithPathTransformer, which seems hard to achieve without duplicating code. It might be possible to get there with multiple inheritance, I haven't tried that yet...)
• I renamed Location with Path as we have numerous arguments path: Location. But maybe location/locn is better.
• Should the subtree identified by the path in the ExprWithPath just be called expr ?
• There was (and is) a function subexps_no_binds(e : Expr) -> List[Expr] that parallels the ExprWithPath method all_subexps(self) -> List[ExprWithPath. Both seem useful (and subexps_no_binds is defined in terms of the other), so it seems they should be named more consistently. Some possibilities to kick off discussion:

ExprWithPath method	toplevel function(Expr)
all_subexps()	subexps_no_binds	(Initial PR)
all_subexprs_with_paths	subexps_no_binds	(Final PR)
subexps_no_binds	subexps_no_binds	Be consistent
subexps_no_binds	subexps_no_binds_expr	But the latter is on exprs!
all_subexprs_with_paths	all_subexp(r)s_that_could_have_paths**

• in that subexprs_with_paths would return List[ExprWithPath]. addressable_subexps, locable_subexps (if Path were called Location)

[awf]

• The Visitor class is a bit messy. It seems useful to have a Visitor-like class for ExprWithPath, and also for Expr (not with path); here these are the same class but they could be two separate classes. (I kept a single ExprVisitor class that handled both, because of the subclass ExprTransformer: if we had separate ExprVisitor and ExprWithPathVisitor, then we would need both ExprTransformer and ExprWithPathTransformer, which seems hard to achieve without duplicating code. It might be possible to get there with multiple inheritance, I haven't tried that yet...)

What are the use cases at the moment for visiting ExprWithPath and Expr? It might be worth listing them here to see if the list gives us a clue as to the design.

• I renamed Location with Path as we have numerous arguments path: Location. But maybe location/locn is better.

Yes, better.

• Should the subtree identified by the path in the ExprWithPath just be called expr ?

Yep, good from my pov.

• There was (and is) a function subexps_no_binds(e : Expr) -> List[Expr] that parallels the ExprWithPath method all_subexps(self) -> List[ExprWithPath. Both seem useful (and subexps_no_binds is defined in terms of the other), so it seems they should be named more consistently. Some possibilities to kick off discussion:

ExprWithPath method toplevel function(Expr)   all_subexps() subexps_no_binds (Current PR) subexps_no_binds subexps_no_binds
subexps_no_binds subexps_no_binds_expr
all_subexprswithpaths subexp(r)s_that_could_have_paths**

• in that subexprs_with_paths would return List[ExprWithPath]. addressable_subexps, locable_subexps (if Path were called Location)

I must admit I'm still kinda hoping these will go away. They currently save some duplication in AbstractMatcher, but I wonder if that might not be better as a visitor anyway?

[acl33]

• The Visitor class is a bit messy. It seems useful to have a Visitor-like class for ExprWithPath, and also for Expr (not with path); here these are the same class but they could be two separate classes. (I kept a single ExprVisitor class that handled both, because of the subclass ExprTransformer: if we had separate ExprVisitor and ExprWithPathVisitor, then we would need both ExprTransformer and ExprWithPathTransformer, which seems hard to achieve without duplicating code. It might be possible to get there with multiple inheritance, I haven't tried that yet...)

What are the use cases at the moment for visiting ExprWithPath and Expr? It might be worth listing them here to see if the list gives us a clue as to the design.

At the moment there are no direct uses of ExprVisitor, only the ExprTransformer subclass. Expr	ExprWithPath
SubstPattern (rewriter)	Capture-avoiding substitution (replace Expr at a path)
UntupleLets

• There was (and is) a function subexps_no_binds(e : Expr) -> List[Expr] that parallels the ExprWithPath method all_subexps(self) -> List[ExprWithPath. Both seem useful (and subexps_no_binds is defined in terms of the other), so it seems they should be named more consistently. Some possibilities to kick off discussion: [snip]

I must admit I'm still kinda hoping these will go away. They currently save some duplication in AbstractMatcher, but I wonder if that might not be better as a visitor anyway?

Currently subexps_no_binds is used in:

• alpha-equivalence for Const, Call, If, Assert;
• hashing for Call, If, Assert
• find_template_subst for Const, Call, If, Assert

All of those probably can be done as Visitors if we gave Visitor a visit_default method or some such, and then we'd call subexps_no_binds from there ;-). Really it exists because from the POV of the rewriter, i.e. dealing with variable binding and not much else, If and Assert work in the same way as Calls like build, fold, map etc.