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

Fix match.As to behave the same as errors.As #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Fryuni
Copy link

@Fryuni Fryuni commented Jan 24, 2022

Fixes #19

@sagikazarmark
Copy link
Member

Thanks @Fryuni ! Can you please take a look at the failing tests?

@Fryuni
Copy link
Author

Fryuni commented Jan 26, 2022

Sure, they all passed locally on 1.17 🙃, I forgot to try on the previous versions

@Fryuni
Copy link
Author

Fryuni commented Jan 26, 2022

I fixed the compatibility problems (the %w formatting), but there is a bigger problem here.

errors.As extract the value in the error chain with the target type and assign it to the target pointer, this is useful to get inner data from the matched error. Writing to the pointer it receives makes it not concurrent-safe. To have the same behavior, match.As would also not be concurrent-safe. Since it is tested now to be concurrent, I think changing it would qualify as a breaking change.

I can remove the race test or change just the matching logic to be the same as errors.As without assigning to the pointer. It seems the latter was the intention of the current code but in that case, I think I should add a match.AsAssign/AsShape or something like that

WDYT @sagikazarmark ?

@Fryuni
Copy link
Author

Fryuni commented Feb 3, 2022

@sagikazarmark just going through my open PRs, any thoughts on this? 😄

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.

match.As not equivalent errors.As
2 participants