-
Notifications
You must be signed in to change notification settings - Fork 485
Add a way to generate callstack trace message via plugin side generation #7148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Added `generate-callstack` option, which, when turned on, will replace every occurances of `PlutusTx.Trace.callStack` with the callstack message at its relative position. This is "rudimentary" because it won't correctly handled when `PlutusTx.Trace.callStack` is called inside of function, call it `bob`, which gets called in multiple locations. It will use callstack when `bob` was called the first time and use that exact callstack in any subsequent calls of `bob` which can be very misleading. This, however, is the best we can get without needing to pass around callstack values at UPLC level or generating multiple copies of `bob` for all of its unique relative positions.
|
Figured out a way to do "real" callstack, with some runtime overhead. This will handle something like this correctly: silly, silly2, silly3, silly4 :: Integer -> Integer
silly x = silly2 x
silly2 x = silly3 x
silly3 x = silly4 x
silly4 x = bob
bob, rob, cob :: Integer
bob = traceError callStack
rob = traceError callStack
cob = traceError callStack
tom :: Integer -> Integer
tom 1 = silly 1
tom 2 = rob
tom 3 = cob
tom _ = 42
|
-- FIXME: Variables will miss span information when the module they are defined | ||
-- in is loaded from cache instead of getting compiled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a workaround for this?
\-nestedLinear:test/Plugin/CallStack/Spec.hs:63:1-63:12 | ||
\-nestedLinear2:test/Plugin/CallStack/Spec.hs:64:1-64:13 | ||
\-nestedLinear3:test/Plugin/CallStack/Spec.hs:65:1-65:13 | ||
\-nestedLinear4:test/Plugin/CallStack/Spec.hs:66:1-66:13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The span provided points to the definition of executed function instead of location of invocation. Is this fine?
... I think it's fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is meant to achieve the goal stated in the issue it is associated with. From the issue description "is useful for determining which part of validator in particular cause the failure." I deduce that goal is for on-chain program authors to be able to determine "which part of a validator causes the failure".
Given two possible implementations:
- One that prints locations of invocations.
- Another that prints locations of definitions.
I'd say that its straightforward how the former one could help authors to determine the failing validator part, and is not straightforward if the latter always helps.
Is there a fundamental reason why we can't have invocation locations?
Please document it in the note, if that's the case.
@@ -0,0 +1,4 @@ | |||
### Added | |||
- `generate-callstack` option: When enabled, the plugin replaces every occurrence of `PlutusTx.Trace.callStack` with a `BuiltinString` containing the callstack at that relative point. Enabling this option incurs execution-cost overhead: it adds an extra callstack argument to all user-defined functions, and every time they are invoked it requires having extra application node for providing the current callstack. When `gennerate-callstack` is not enabled, `PlutusTx.Trace.callStack` is string "<CallStack>". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `generate-callstack` option: When enabled, the plugin replaces every occurrence of `PlutusTx.Trace.callStack` with a `BuiltinString` containing the callstack at that relative point. Enabling this option incurs execution-cost overhead: it adds an extra callstack argument to all user-defined functions, and every time they are invoked it requires having extra application node for providing the current callstack. When `gennerate-callstack` is not enabled, `PlutusTx.Trace.callStack` is string "<CallStack>". | |
- `generate-callstack` option: When enabled, the plugin replaces every occurrence of `PlutusTx.Trace.callStack` with a `BuiltinString` containing the callstack at that relative point. Enabling this option incurs execution-cost overhead: it adds an extra callstack argument to all user-defined functions, and every time they are invoked it requires having extra application node for providing the current callstack. When `generate-callstack` is not enabled, `PlutusTx.Trace.callStack` is string "<CallStack>". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! LGTM, with the caveat that I'm not very familiar with the code in Expr
. Maybe somebody else should approve as well before you merge.
} | ||
|
||
lastCallStackName :: MonadState CompileState m => m (Maybe (PIR.Name)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lastCallStackName :: MonadState CompileState m => m (Maybe (PIR.Name)) | |
lastCallStackName :: MonadState CompileState m => m (Maybe PIR.Name) |
@@ -309,6 +310,9 @@ pluginOptions = | |||
rest <- many (alphaNumChar <|> char '_' <|> char '\\') | |||
pure (firstC : rest) | |||
in (k, PluginOption typeRep (plcParserOption p k) posCertify desc []) | |||
, let k = "generate-callstack" | |||
desc = "Generate callstack string and replace 'callStack' with callstack at the location." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the "replace 'callStack' with callstack at the location." part mean for a compiler user?
I don't understand it myself, don't think that end-users will get it either.
\-nestedLinear2:test/Plugin/CallStack/Spec.hs:64:1-64:13 | ||
\-nestedLinear3:test/Plugin/CallStack/Spec.hs:65:1-65:13 | ||
\-nestedLinear4:test/Plugin/CallStack/Spec.hs:66:1-66:13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you use \
backslash? I'd find it more useful to have some notion of a direction here: should I read it top to bottom or vice-versa? Maybe 1. 2. 3. is better then?
superseded by #7178 unless someone actually prefers this method |
closes #7146.
Add plugin option to generate callstack trace message. Currently this won't handle cases when a function hasPlutusTx.Trace.callStack
and that function is called in multiple places. It will just use callstack of the first occurance :/ (working on it. more detail on first commit message).I redid everything, now it will have correct callstack, but with some execution cost overhead. I'm not too unhappy about this since callstack in general is almost exclusively used for debug scripts anyways.
Example