fix[lang]: show user error in error map #4286
Conversation
| raise "some error" | ||
| """ | ||
| error_map = compile_code(code, output_formats=["source_map"])["source_map"]["error_map"] | ||
| assert "user revert with reason" in list(error_map.values()) |
There was a problem hiding this comment.
why can't we check in error_map.values()?
There was a problem hiding this comment.
We can. test_error_map also wraps the values in list - should I change it there too?
There was a problem hiding this comment.
it's a dict view, i think we should. @charles-cooper do you remember the reason you did it this way?
There was a problem hiding this comment.
it's a dict view, i think we should. @charles-cooper do you remember the reason you did it this way?
no, just habit i think
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4286 +/- ##
===========================================
- Coverage 91.27% 45.57% -45.71%
===========================================
Files 112 112
Lines 16017 16017
Branches 2696 2696
===========================================
- Hits 14619 7299 -7320
- Misses 966 8159 +7193
- Partials 432 559 +127 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
vyper/codegen/stmt.py
Outdated
| return IRnode.from_list(ir_node, error_msg="user revert with reason") | ||
|
|
||
| ir_node = IRnode.from_list(ir_node) | ||
| ir_node.set_error_msg("user revert with reason") |
There was a problem hiding this comment.
what if some of the arguments have their own error message? Then it gets overwritten although it might be more specific.
eg in the case of raise self.bar(5 % i) the mod is annotated with a safemod error msg, which will be replaced with user revert with reason
There was a problem hiding this comment.
Hmm I did implement set_error_msg to override, but that could be changed. @sandbubbles could you change that in this PR to not override? And add @cyberthirst's test case raise self.foo(5%i) to the test suite.
There was a problem hiding this comment.
does it make sense actually to just call set_error_msg in from_list()? since i imagine the same issue may happen at other call sites?
vyper/codegen/ir_node.py
Outdated
| is_self_call=is_self_call, | ||
| passthrough_metadata=passthrough_metadata, | ||
| ) | ||
| if error_msg is not None: | ||
| ret.set_error_msg(error_msg) |
There was a problem hiding this comment.
maybe _set_error_msg, since the IRnode may already have error_msg set
What I did
Fix #4199
How I did it
Moved error message to "revert"
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture