Gen hackery

[endgame]

I had hoped to teach gen how to handle recursive shape definitions in general, but ran out of time and mainline project work re-starts from next week. This at least lets us regenerate everything, as previously rds-data failed to generate due to a three-deep recursive cycle.

This PR teaches gen to handle recursive shape definitions.

[endgame]

@brendanhay I tried generating a Ptr every time we see a type that we've seen before, basically by generalising the condition on the below line to n `elem` seen:

https://github.com/brendanhay/amazonka/blob/f73a957d05f64863e867cf39d0db260718f0fadd/gen/src/Gen/AST/Cofree.hs#L65

When I tried to generate bindings for dynamodb (it's a service I know reasonably well), it failed to generate lens prefixes on labels, like what happened when you tried to generate kendra bindings (reported at #730). This makes me think that there's some rewriting pass or something that doesn't add the prefixes correctly if it sees a Ptr instead of a proper recursive shape; do you have any insights?

Otherwise, I hope we can at least turn the handle on some of the new services, and get a big regenerate in sometime in the next few weeks.

[RobertFischer]

We are going to get rid of the lens stuff, right? So maybe just sit on this until then?

On Fri, Mar 11, 2022, 01:15 endgame @.***> wrote:

@brendanhay https://github.com/brendanhay I tried generating a Ptr every time we see a type that we've seen before, basically by generalising the condition on the below line to n elem seen:

https://github.com/brendanhay/amazonka/blob/f73a957d05f64863e867cf39d0db260718f0fadd/gen/src/Gen/AST/Cofree.hs#L65

When I tried to generate bindings for dynamodb (it's a service I know reasonably well), it failed to generate lens prefixes on labels, like what happened when you tried to generate kendra bindings (reported at #730 https://github.com/brendanhay/amazonka/issues/730). This makes me think that there's some rewriting pass or something that doesn't add the prefixes correctly if it sees a Ptr instead of a proper recursive shape; do you have any insights?

Otherwise, I hope we can at least turn the handle on some of the new services, and get a big regenerate in sometime in the next few weeks.

— Reply to this email directly, view it on GitHub https://github.com/brendanhay/amazonka/pull/761#issuecomment-1064807625, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAE5VZFRMFHYQMTJM7NK33U7LQHDANCNFSM5QOWCBEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

[endgame]

If you mean the PR as a whole, no: we need to be able to regen services.

If you mean the Ptr stuff, then I still think that's unwise: I think there's probably something else afoot that we ignore at our peril. AIUI, the eventual plan for lens is to avoid the lens dependency (so callers can use either generic-lens or generic-optics, but still provide VL lenses for data types under unambiguous names.

[brendanhay]

Need to run this and poke around to rebuild my memory index as nothing concrete comes to mind.

I suspect your intuition is correct, ie. elaborate from the flat JSON parse result (shape) -> recursive cofree, then probably only rename the top-level without descending into the recursive shape to rename all references, or something similarly stupid.

[endgame]

That would be excellent. I wasn't able to grok that part of the generator, so if you get a chance to test my theory, I'd be very grateful.

[endgame]

@brendanhay Starting next week, I should have a couple of work weeks where I can give amazonka some more attention. If you can get a chance to look at the generator before then, I'd really appreciate it. If not, then I intend to merge this to unblock things (we can't gen right now for some services we previously could; I suspect the change to hash functions in hashable ^>=1.4) and see what i can get done.

[endgame]

@brendanhay Some friends and I found what we believe to be a subtle bug in the way the seen list was being tracked inside elaborate: when recursing into refs, the refs would add themselves to the seen list. If the root object adds itself to seen, then it looks like we can unconditionally generate Ptr when we hit a shape that we've seen before. Because seen is cleared each time we explore a value in the input HashMap, we guarantee that we never memoise a Ptr and that the top level structures are always StructF, so generation still proceeds.

It would be great if you could review b3985cd347877f4dbaf06c65bf4049a5ea1eacc8 in particular, but I'm happy to merge and get back to fixing services if you're busy.

[endgame]

When I tried to generate bindings for dynamodb (it's a service I know reasonably well), it failed to generate lens prefixes on labels, like what happened when you tried to generate kendra bindings (reported at #730)...

This appears to be fixed, too.

[endgame]

For some libraries with recursive definitions (e.g., wafv2; #736), this version of gen successfully generates code, but the generated library will not build thanks to a module import cycle. Should we generate .hs-boot files for any shapes where we had to create a Ptr?

[endgame]

Notes for myself, for when I next get a chance to hack on this:

I think also it might not be too bad:

• when writing out a type file, if the shape you need to import is a Ptr, do an import {-# SOURCE #-}
• when building the tree of files to write out, generate .hs-boot for files which are referenced at least once by Ptr