Skip to content
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

Workaround for #34 #35

Merged
merged 20 commits into from
Aug 13, 2024
Merged

Workaround for #34 #35

merged 20 commits into from
Aug 13, 2024

Conversation

arnetheduck
Copy link
Owner

@arnetheduck arnetheduck commented Sep 4, 2023

This PR adds a workaround for the case where a global symbol is matched instead of the local error/value template in valueOr and friends, when valueOr is being used in a generic context.

Two options are added:

  • when using Nim version supporting the experimental genericsOpenSym feature, we use it
  • when not, a macro tries to replace all accesses in the given body with the correct symbol - this matching is done by name and some minimal heuristics which potentially could be wrong - the upstream solution is obviously preferable

Both solutions can be disabled via compile-time options to retain the old behavior of matching the global symbol.

See also nim-lang/Nim#22605

thanks to @Araq for the macro hack and @metagn for the language fix!

results.nim Outdated Show resolved Hide resolved
@etan-status
Copy link
Contributor

Related: status-im/nim-stew#174

@etan-status
Copy link
Contributor

How well does this work when passing error as a kv param to a chronicles log? will the hack become user visible?

@etan-status
Copy link
Contributor

etan-status commented Sep 6, 2023

Also, how big is the desire to have any workaround within the status repos, if we rename the injected name to err instead of error? How about resultErr or resError?

@arnetheduck
Copy link
Owner Author

How well does this work when passing error as a kv param to a chronicles log? will the hack become user visible?

works fine - this is actually where the problem manifests in our code: an error enum gets logged as "error" instead of the actual error we want, due to the automatic string conversion that logging does

@arnetheduck arnetheduck merged commit 410afe8 into master Aug 13, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants