Support mkNil for any builtin types
#7438
Conversation
SeungheonOh
changed the title
mkNil for Bool and BuiltinByteStringmkNil for any builtin types
|
I made it so that now |
effectfully
left a comment
effectfully
left a comment
There was a problem hiding this comment.
You're my hero.
This was my original idea on how to make it work in the general case:
The only more or less reliable way I could think of is serializing the type at the type level into a
Symbol(to make sure it's fully done at compile time), requiring it to beKnownSymbol, then extracting the resultingStringin the compiler and parsing it back to get aDefaultUniobject. Sounds pretty crazy, but I can't think of another way of parsing a Haskell value back after embedding it into aCoreExpr.
But that's a lot of effort for something rather insignificant and your solution should be enough in most cases (I don't wanna think when it's not enough).
I'll check if I can it statically detectable that an instance is missing.
kwxm
left a comment
kwxm
left a comment
There was a problem hiding this comment.
This is great: it makes the behaviour of the compiler much more consistent. I don't understand why it works, but I tried some examples and it was able to cope with all of them.
It might be worth adding some tests for the new capabilities before merging. Also, is there any documentation that will need to be updated? Possibly we've said somehere that mkNil doesn't work in full generalilty, but I don't know where to look for that kind of thing.
| | tyCon == builtinBLS12_G1_TyCon = pure $ SomeTypeIn' PLC.DefaultUniBLS12_381_G1_Element | ||
| | tyCon == builtinBLS12_G2_TyCon = pure $ SomeTypeIn' PLC.DefaultUniBLS12_381_G2_Element | ||
| | tyCon == builtinValueTyCon = pure $ SomeTypeIn' PLC.DefaultUniValue | ||
| | otherwise = Nothing |
There was a problem hiding this comment.
This worries me a little because we can add a new built-in type and forget to add it here, and we won't get any kind of warning from the compiler. I don't immediately see any way of avoiding that though. Maybe we could have a warning comment somewhere that you'd see it when you're adding a new type, for example at the definition of mkNil or DefaultUni ? Or could we have a test that iterates over everything in DefaultUni and checks that mkNil works for the Plinth version?
There was a problem hiding this comment.
I don't really have a concern here since this won't really generate any unsafe code. Adding a comment on class definition of MkNil would be sufficient imo.
| mkNil = mkNilOpaque | ||
| {-# INLINEABLE mkNil #-} | ||
| instance MkNil BuiltinInteger | ||
| instance MkNil BuiltinByteString |
There was a problem hiding this comment.
I tried adding an instance for BuiltinArray here and then mkNil does indeed work for the improbable case of lists of arrays. Also, since we're here, the first lot of parentheses can be deleted in
instance (MkNil a) => MkNil (BuiltinList a)
a few lines later.
I've scanned over the repo, didn't find anything that mention |
You can now use
PlutusTx.Builtins.Opaque.mkNil @BoolandmkNil @BuiltinByteString.This doesn't add support for arbitrarily nested array. Only adds support for
BoolandByteString.I'm figuring out making this work with any subtype of default universe, so please don't merge yet