Add `makeIsDataAsList` for generating `IsData` instances that uses `List` internally #7121

SeungheonOh · 2025-05-28T18:10:02Z

PlutusTx.IsData.TH in general is rather a mess; there's a ton of unnecessary "helpers" and obfuscation. It would be nice to redo parts of it from scratch since now I introduced a bit of code duplication on top. But perhaps in a separate refactoring PR.

I'll add some more test cases tomorrow

SeungheonOh · 2025-05-28T18:11:00Z

plutus-tx-plugin/test/IsData/Spec.hs

@@ -158,6 +164,7 @@ tests =
      , goldenUEval "mono" [plc (Proxy @"mono") (isDataRoundtrip (Mono2 2))]
      , goldenUEval "poly" [plc (Proxy @"poly") (isDataRoundtrip (Poly1 (1 :: Integer) (2 :: Integer)))]
      , goldenUEval "record" [plc (Proxy @"record") (isDataRoundtrip (MyMonoRecord 1 2))]
+      , goldenUEval "recordAsList" [plc (Proxy @"record") (isDataRoundtrip (MyMonoRecordAsList 1 2))]


Need something better than this since goldenUEval doesn't check if given value actually is True value.

Let me play a "Devil's advocate":
Now, as the golden file containing a representation of True is in place, you have your check -
If only it starts producing False - you'd get a golden test failure.

Still bad 😅

Is your concern that constr 0 [] could represent something else other than True? True, but highly unlikely.

It should be okay for now, but we've seen representation of Bool changing(with introduction of SOP) and also more importantly, there's no reason to have this as golden at all! Golden from these cases are just purely unnecessary noises

I'll make some changes

Unisay · 2025-05-29T12:43:08Z

plutus-tx/src/PlutusTx/IsData/TH.hs

+    -- Do not change this to raw integer pattern matching.
+    -- GHC will inline `equalsEq` and do integer equality by matching
+    -- them structurally, which the plugin does not know how to handle.


Valuable insight 👍🏼

Unisay · 2025-05-29T12:45:34Z

plutus-tx-plugin/test/IsData/Spec.hs

+instance P.Eq MyMonoRecordAsList where
+  {-# INLINEABLE (==) #-}
+  (MyMonoRecordAsList x1 y1) == (MyMonoRecordAsList x2 y2) = x1 P.== x2 && y1 P.== y2
+IsData.makeIsDataAsList ''MyMonoRecordAsList


I wonder if its possible to capture resulting syntax in a golden file 🤔

Yes, do we want that?

@zliu41 wdyt?

Yes - because otherwise there's no evidence that makeIsDataAsList actually uses the List constructor. The existing tests would succeed if it was using Constr.

zliu41

This needs a changelog - it's a user-facing change. Also needs a golden test showing that it uses the List constructor. Otherwise looks good.

plutus-tx/src/PlutusTx/IsData/TH.hs

zliu41 · 2025-06-01T23:21:44Z

plutus-tx-plugin/test/IsData/Spec.hs

@@ -158,6 +164,7 @@ tests =
      , goldenUEval "mono" [plc (Proxy @"mono") (isDataRoundtrip (Mono2 2))]
      , goldenUEval "poly" [plc (Proxy @"poly") (isDataRoundtrip (Poly1 (1 :: Integer) (2 :: Integer)))]
      , goldenUEval "record" [plc (Proxy @"record") (isDataRoundtrip (MyMonoRecord 1 2))]
+      , goldenUEval "recordAsList" [plc (Proxy @"record") (isDataRoundtrip (MyMonoRecordAsList 1 2))]


Is your concern that constr 0 [] could represent something else other than True? True, but highly unlikely.

zliu41 · 2025-06-01T23:24:02Z

plutus-tx-plugin/test/IsData/Spec.hs

+instance P.Eq MyMonoRecordAsList where
+  {-# INLINEABLE (==) #-}
+  (MyMonoRecordAsList x1 y1) == (MyMonoRecordAsList x2 y2) = x1 P.== x2 && y1 P.== y2
+IsData.makeIsDataAsList ''MyMonoRecordAsList


Yes - because otherwise there's no evidence that makeIsDataAsList actually uses the List constructor. The existing tests would succeed if it was using Constr.

…ist` internally

SeungheonOh · 2025-06-02T12:43:50Z

plutus-tx/testlib/PlutusTx/Test/Util/Apply.hs

+compiledCodeToHask foo bar baz :: Either String (CompiledCode ())
+```
+-}
+compiledCodeToHask


@Unisay I made this cute little helper. You might be interested; no need for spamming unsafeApplyCode.

I like ergonomics of it but not sure about semantics: does it translate to the Apply constructor in UPLC?

this is just fancy applyCode and unsafeApplyCode, so it will behave identically to these, which iirc will create Apply node on UPLC.

Perhaps name can be made less confusing. My reasoning for compiledCodeToHask was that it lifts functions in UPLC(CompiledCode (a->b->c)) into functions in Hask(CompiledCode a -> CompiledCode b -> CompiledCode c).

If it has the same semantics (we're not substituting UPLC application with Haskell application) then I support it!

zliu41

@SeungheonOh Sorry for the confusion but I didn't mean showing the TH splices in the golden files. You can make a pir.golden compiled from converting a small data type to Data, which would show that it uses the List constructor, and that's much simpler than the TH splices.

plutus-tx/testlib/PlutusTx/Test/Run/Code.hs

SeungheonOh · 2025-06-02T16:34:10Z

@zliu41 Okay, I'll add pir goldens too. But, I think TH splice goldens are still be very helpful as a quick reference point to see how codegen currently behaves. It would be especially useful when we need to make changes later on and my memories on how generated code looks like got faded away.

SeungheonOh temporarily deployed to github-pages May 28, 2025 18:10 — with GitHub Actions Inactive

SeungheonOh commented May 28, 2025

View reviewed changes

SeungheonOh requested a review from Unisay May 28, 2025 18:11

SeungheonOh added the No Changelog Required Add this to skip the Changelog Check label May 28, 2025

Unisay reviewed May 29, 2025

View reviewed changes

zliu41 removed the No Changelog Required Add this to skip the Changelog Check label Jun 1, 2025

zliu41 approved these changes Jun 1, 2025

View reviewed changes

Add makeIsDataAsList for generating IsData instances that uses `L…

d559ec1

…ist` internally

SeungheonOh force-pushed the sho/7115 branch from 4df0170 to bb1de3b Compare June 2, 2025 12:41

SeungheonOh temporarily deployed to github-pages June 2, 2025 12:41 — with GitHub Actions Inactive

SeungheonOh force-pushed the sho/7115 branch from bb1de3b to 5f4c654 Compare June 2, 2025 12:42

SeungheonOh temporarily deployed to github-pages June 2, 2025 12:42 — with GitHub Actions Inactive

SeungheonOh commented Jun 2, 2025

View reviewed changes

SeungheonOh force-pushed the sho/7115 branch from 5f4c654 to dbeca1e Compare June 2, 2025 12:50

SeungheonOh temporarily deployed to github-pages June 2, 2025 12:50 — with GitHub Actions Inactive

SeungheonOh added 2 commits June 2, 2025 22:03

Add new testing utilities, add th codegen tests, remove noise on goldens

e21f8d3

Add changelog

98c806e

SeungheonOh force-pushed the sho/7115 branch from dbeca1e to 98c806e Compare June 2, 2025 13:04

SeungheonOh temporarily deployed to github-pages June 2, 2025 13:04 — with GitHub Actions Inactive

zliu41 requested changes Jun 2, 2025

View reviewed changes

Unisay reviewed Jun 2, 2025

View reviewed changes

plutus-tx/testlib/PlutusTx/Test/Run/Code.hs Outdated Show resolved Hide resolved

Add PIR golden and rename assertResult

3bc7c46

SeungheonOh temporarily deployed to github-pages June 2, 2025 17:52 — with GitHub Actions Inactive

zliu41 approved these changes Jun 2, 2025

View reviewed changes

zliu41 merged commit 8626d8f into master Jun 2, 2025
6 of 7 checks passed

zliu41 deleted the sho/7115 branch June 2, 2025 20:20

Add makeIsDataAsList for generating IsData instances that uses List internally #7121

Add makeIsDataAsList for generating IsData instances that uses List internally #7121

Uh oh!

Conversation

SeungheonOh commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zliu41 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zliu41 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SeungheonOh commented Jun 2, 2025

Uh oh!

Uh oh!

Uh oh!

Add `makeIsDataAsList` for generating `IsData` instances that uses `List` internally #7121

Add `makeIsDataAsList` for generating `IsData` instances that uses `List` internally #7121

SeungheonOh commented May 28, 2025 •

edited

Loading