Make hiffy_decode return a non-nested error#661
Merged
Conversation
e0a515b to
2dea381
Compare
70f547e to
02dc755
Compare
2dea381 to
861ffd1
Compare
02dc755 to
27e0c3a
Compare
861ffd1 to
c3de5fa
Compare
27e0c3a to
1e0ed19
Compare
c3de5fa to
e66628c
Compare
1e0ed19 to
a35dcb6
Compare
e66628c to
a9801cb
Compare
99df9fd to
effa762
Compare
61239ac to
1fe703a
Compare
effa762 to
0b5c70a
Compare
4a5d1b2 to
a25af4c
Compare
0b5c70a to
607b95e
Compare
labbott
approved these changes
May 20, 2026
Contributor
labbott
left a comment
There was a problem hiding this comment.
Minor question about performance but it looks correct
Comment on lines
+749
to
+759
| let cfg = s.field::<Struct>("cfg")?; | ||
| assert_eq!(cfg.name(), "PortConfig"); | ||
| let dev = decode_dev(&cfg.field("dev")?)?; | ||
| let serdes = decode_dev(&cfg.field("serdes")?)?; | ||
| let (mode, speed) = decode_mode(&cfg["mode"]); | ||
| ( | ||
| dev.replace("DEV", ""), | ||
| serdes.replace("SERDES", ""), | ||
| mode, | ||
| speed, | ||
| ) |
Contributor
There was a problem hiding this comment.
How expensive is each .field call?
Contributor
Author
There was a problem hiding this comment.
It's a lookup in a hashmap and then a Load call (basically the same as before, the hashmap lookup was previously hidden in the indexing operation).
a25af4c to
f59ccb1
Compare
b28c3a5 to
f98a018
Compare
84759c9 to
46713c4
Compare
f98a018 to
82dd66e
Compare
46713c4 to
7e4fe3f
Compare
82dd66e to
1d7bdca
Compare
7e4fe3f to
d72d23e
Compare
1d7bdca to
ced5065
Compare
d72d23e to
fdfe094
Compare
ced5065 to
ff30bea
Compare
fdfe094 to
88959bc
Compare
ff30bea to
38483a1
Compare
88959bc to
ee01fe0
Compare
e6a5158 to
70d7c6e
Compare
ee01fe0 to
22fae46
Compare
70d7c6e to
649d9d4
Compare
22fae46 to
4718de5
Compare
649d9d4 to
c120380
Compare
36ecdb0 to
5171b49
Compare
c120380 to
f1b8bcc
Compare
e722703 to
bfd295e
Compare
bfd295e to
24d7716
Compare
Contributor
Author
|
Quick test sweep over affected commands: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(staged on #660)
hiffy_decodereturns aResult<std::result::Result<T, String>>, which is an awkward type.We can instead use the
HiffyCallError(renamedHiffyError), which distinguishes between internal errors (returned by the HIF program) and external errors (which occur when calling hiffy).This simplifies most cases (where we want to return both inner and outer error), and adds a tiny bit of boilerplate to cases where we care about the difference – but that boilerplate is more clear than the nested type.