IntersectMBO / plutus

The Plutus language implementation and tools
Apache License 2.0
1.54k stars 468 forks source link

"Debug symbols" for plutus-tx #5194

Open shlevy opened 1 year ago

shlevy commented 1 year ago

Describe the feature you'd like

Would be great if the compiler produced 2 artifacts, one version of the contract with all debugging code-paths stripped out to be used on-chain and one with debugging kept in for off-chain usage.

This goes beyond remove-trace in two ways:

  1. Both artifacts are created at once, so there's no need to call with a different set of plugin options
  2. User extensible definition of "debugging code path" (described below)

I think there are 2 kinds of cases where we'd want to specify something as a "debug code path" (perhaps telling the compiler this through annotations?)

First is "unconditional", where any time some term appears it is unconditionally replaced with some other term. trace gets replaced with flip const.

Second is "conditional", where we use some special function to indicate a promise that some subset of the codomain corresponds to failure-case debugging. E.g. debugFailure (fromBuiltinData d) :: Maybe a would get rewritten to (Just . unsafeFromBuiltinData) d :: Maybe a in prod mode (and debugFailure would act as id otherwise), and the user is promising that in the Nothing case there will just be some tracing followed, ultimately, by a call to error.

Describe alternatives you've considered

Status quo, basically.

michaelpj commented 1 year ago

I'm unsure about 1. Note that users can do this fairly easily themselves:

module Module1 where

myCode :: CompiledCode Integer
myCode = $$(compile whatever)

{-# OPTIONS_GHC -fplugin-opt ... -#}
module Module2 where

myCodeDebug :: CompiledCode Integer
myCodeDebug = $$(compile whatever)

The main advantage of adding 1 would be to be able to do this in one module, I think. Nothing downstream will understand a multi-script with different options, so you're going to have to pass on the specific one that you want pretty quickly.

The downside is that it's not clear why we should prefer this exact combination of flags. What if people also want to run more optimizations on the "release" version? Better to just let the user do what they want IMO.


I think we could do 2 in a nice way that subsumes remove-trace:

  1. Add a debug flag (bikeshedding here)
  2. Add a function ifDebug :: a -> a -> a, which is compiled to \x y . x if debug is set and \x y . y if it is not

Now we could define debugTraceError message = ifDebug (\( ) -> traceError message) (\() -> error) (). Similalrly, sometimesUnsafeFromBuiltinData d = ifDebug (\() -> fromBuiltinData d) (Just (unsafeFromBuiltinData d)) ().

This isn't quite the same as traceError plus remove-trace, but it's pretty similar. You use the debugTrace variants for anything you want to remove on-chain, and you can use traceError directly for stuff that you want to remain.

However... I notice that this is again definable in user-space. You can define ifDebug as a normal function that has different behaviour based on a CPP flag or a TH macro that can access all kinds of stuff. So maybe we should try and do it that way :thinking: remove-trace has some justification for being in the plugin, since it actually murders the builtin function deep down in the compiler. Once again I am unsure.

shlevy commented 1 year ago

Is there a reason we pass arguments through f-plugin-opts instead of an argument to compile?

michaelpj commented 1 year ago

Not sure how to get things into a plugin that way. You'd have to take the Core for the argument and then somehow turn it into a Haskell value. Let's not even talk about cross.

michaelpj commented 1 year ago

Oh, we do pass some information with type-level string arguments (namely the location), but it's quite annoying.

shlevy commented 1 year ago

Hmm, OK. So I guess I could write all my Plutus code in (something with a nicer interface implemented on top of) a ReaderT (forall a. a -> a -> a) and then simply have two calls to compile passing in a different ifDebug. I'll try that.

shlevy commented 1 year ago

I can close if the guidance is that we should not expect this to end up in the compiler or stdlib, but if there's a chance something like this could be upstreamed I'll keep this open and report back.

shlevy commented 1 year ago

Nothing downstream will understand a multi-script with different options

If we build it, they will come :stuck_out_tongue_winking_eye:

michaelpj commented 1 year ago

Hmm, OK. So I guess I could write all my Plutus code in (something with a nicer interface implemented on top of) a ReaderT (forall a. a -> a -> a) and then simply have two calls to compile passing in a different ifDebug. I'll try that.

That won't work. We won't be able to work out what the function is when we try and compile things. You need something that is statically one version of ifDebug or the other. Hence my suggestion to use CPP, but if you want both versions you might need to do something with TH. I'm not sure if there's a sensible way to have a TH splice that queries options that can be different per-module...

Indeed, having written that I realise that what I suggested is in fact impossible and we need to do it in the compiler. The reason is that you want to have the same code be compiled by the plugin two different ways, depending on the compile call. I was suggesting that we get GHC to do the different compilation - but of course, GHC will compile each bit of code once. So it needs to happen in the plugin's processing of that code, so we're back to a plugin option.

Okay, so I'm tentatively in favour of a debug flag that does what I described above. I kind of want to make it more generic but maybe that's pushing it. E.g. -fplugin-opt PlutusTx.Plugin:user-opt=debug and then we have a magic function queryOpt :: forall (opt :: Symbol) . BuiltinBool that looks at the type-level string to decide what to put in. ifDebug x y = ifThenElse (queryOpt @"debug") x y.


If we build it, they will come stuck_out_tongue_winking_eye

YAGNI? Are we playing the aphorism-swapping game? :P

shlevy commented 1 year ago

We won't be able to work out what the function is when we try and compile things... GHC will compile each bit of code once

Hmm, I'm confused. Given this example in the docs:

integerIdentity :: CompiledCode (Integer -> Integer)
integerIdentity = $$(compile [|| \(x:: Integer) -> x ||])

I'd assumed you could do something like $$(compile [|| foo True ||]) and then the first argument to foo would be "statically known" from the perspective of the plugin (i.e. we'd compile a new expression which would inline foo and then reduce if the compiler thinks it expedient). Is that wrong?

I kind of want to make it more generic but maybe that's pushing it

I like what you're sketching here.

michaelpj commented 1 year ago

I'd assumed you could do something like $$(compile [|| foo True ||]) and then the first argument to foo would be "statically known" from the perspective of the plugin (i.e. we'd compile a new expression which would inline foo and then reduce if the compiler thinks it expedient). Is that wrong?

Yes, but what I thought you were proposing was more like this (perhaps I misunderstood):

withIfDebug :: (forall a . a -> a -> a) -> CompiledCode Integer
withIfDebug f = $$(compile [|| f 1 2 ||])

That won't work, because we don't know what f is.

shlevy commented 1 year ago

I was proposing something like:


scriptCode :: DebugMode -> a

prodAndDebugScript :: (CompiledCode a, CompiledCode a)
prodAndDebugScript = ($$(compile [|| scriptCode NoDebug ||]), $$(compile [|| scriptCode Debug ||]))
shlevy commented 1 year ago

And maybe we even write a TH function compileProdAndDebug

michaelpj commented 1 year ago

That might work out yeah, but you'd be very reliant on us doing a lot of inlining to eliminate things all the way down to the final ifDebug. And it would be pretty bad UX.

shlevy commented 1 year ago

So to make sure I understand your proposal, queryOpt @"Foo" would be noinline (and presumably its definition would be BuiltinBool False), and then the plugin would, as an early pass, replace each instance of queryOpt with the actual bool value based on the flags at the call to compile, and then the rest of the compiler passes would constant-reduce the relevant branches?

shlevy commented 1 year ago

Should it really be BuiltinBool though? Don't we want the user's code to directly pattern-match on it, not use the on-chain bool case function?

michaelpj commented 1 year ago

Yep, you got it. We have a special error to use as the body in that case, mustBeReplaced. And a special case in compileExpr.

Should it really be BuiltinBool though? Don't we want the user's code to directly pattern-match on it, not use the on-chain bool case function?

The plugin doesn't know about the existence of Bool, but it does know about BuiltinBool. We can do the conversion to Bool in userspace and let the optimizer deal with it. We don't currently have a pass to reduce ifThenElse CONSTANT x y but we should and it's easy to write.

shlevy commented 1 year ago

OK so the user-facing interface would be queryOpt :: forall (opt :: Symbol) . Bool, which would be implemented on top of Builtins.Internal.queryOpt :: forall (opt :: Symbol) . BuiltinBool as ifThenElse (Internal.queryOpt @opt) True False?

shlevy commented 1 year ago

I'm assuming we can't just use defineBuiltinTerm directly since we'll need to somehow inspect the core for which symbol is being requested, right?

michaelpj commented 1 year ago

It would need a special case in compileExpr, I think, to match on a type application of queryBool to a known string.

effectfully commented 1 year ago

@michaelpj so are we interested in providing this functionality? If so, may I ask to to create a Jira ticket for it with all the details, so that we can prioritize this work through the usual means?

effectfully commented 1 year ago

This doesn't appear to be a high-priority ticket, hence I'm marking it with "low priority". If anybody disagrees, please do remove the label and we'll reevaluate the issue.

michaelpj commented 1 year ago

I think Shea might do it, but yes I'll put it into JIRA.

michaelpj commented 1 year ago

PLT-4585

effectfully commented 1 month ago

I think Shea might do it, but yes I'll put it into JIRA.

There's no Jira anymore, so I'm copying a comment that was moved from Jira to a separate GitHub issue here:

The interface we were settling on was something like this:

Add a special function queryUserFlag (forall opt :: Symbol) :: BuiltinBool

The compiler should recognize this function (we can’t compile it normally anyway because of the type-level strings), and compile it into true/false depending on whether the corresponding -fplugin-opt PlutusTx.Plugin:user-flag=whatever is set

(name bikeshedding possible of course)

A natural use of this is for “debug logging”: we can define

debugTrace :: BuiltinString -> a -> a
debugTrace s x = ifThenElse (queryOpt @"debug") (\_ -> trace s x) (\_ -> x) ()

More generally it enables user-controlled compile-time flags, which is quite handy.

Notes:

Note that we can also do this for some other builtin types, e.g. we could have user-defined integers or strings fairly easily. Unsure how useful that would be.

We will want PLT-4586: Simple optimization for ifThenElse also to ensure that the code in the dead branch gets eliminated.