CozySynthesizer / cozy

The collection synthesizer
https://cozy.uwplse.org
Apache License 2.0
209 stars 18 forks source link

Change `.t` to `.elem_type` #45

Closed Calvin-L closed 6 years ago

Calvin-L commented 6 years ago

The classes that represent collection types (TBag, TSet, and TList) use the attribute t for the type of their elements. This is ugly and is a bad name.

We should rename t to elem_type. This will be a wide-reaching change and we should be careful we got it right. Python's untyped nature makes it tricky.

Similarly, we might also want to perform these other renamings:

(This is a follow-up to #43 and #44.)

anhnamtran commented 6 years ago

Do you just want this changed on TBag, TSet, and TList? There's also t in TApp, TRef, and TVector. Also TTuple has a ts and a n in it as well.

Calvin-L commented 6 years ago

Yes, those should probably also get cleaner names.

(Also, I am not sure TVector is used anymore.)

anhnamtran commented 6 years ago

I'll take a look at TVector and do some cleaning up.

anhnamtran commented 6 years ago

Similarly, are most of the .fs .fields as well? I was looking through it and it seems to make sense. Since they're in EArgMin and so on...

Calvin-L commented 6 years ago

No, most of the fs are functions. So perhaps:

anhnamtran commented 6 years ago

When I change EArg(Min|Max).f to .key_function, EMap and EFlatMap breaks. For now I will temporarily change their .f to .key_function as well.

Followup: This is because in some places where you're checking isinstance(_, _) there are groups of EArg(Min|Max) or EFlatMap or EMap.

Calvin-L commented 6 years ago

Yep, those isinstance checks are hacky. They exploit the fact that all of those AST nodes have .e and .f properties. You should fix them by duplicating those checks and their bodies for each different case.

I agree that, too, feels wrong. We should think about a better design for the places that happens.

anhnamtran commented 6 years ago

Everything should be covered in #47.