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.

Sign up for GitHub

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

Jump to bottom

proposal: errors: add AsOf to the API #56949

Closed
phenpessoa opened this issue Nov 27, 2022 · 7 comments
Closed

proposal: errors: add AsOf to the API #56949

phenpessoa opened this issue Nov 27, 2022 · 7 comments
Labels
FrozenDueToAge Proposal
Milestone
Proposal

Comments

@phenpessoa
Copy link

phenpessoa commented Nov 27, 2022 •
edited

With the introduction of generics, I think it makes sense to have an AsOf function in the errors package that will return the wanted error.
The API would look like this:

func AsOf[E error](err error) (E, bool)

A simple implementation of this func could be

func AsOf[E error](err error) (E, bool) {
	e := new(E)
	ok := As(err, e)
	return *e, ok
}

But, with generics, we can have this be implemented without reflection and with an allocation free path

func AsOf[E error](err error) (E, bool) {
	var ptrToE *E
	for err != nil {
		if e, ok := err.(E); ok {
			return e, true
		}
		if x, ok := err.(interface{ As(any) bool }); ok {
			if ptrToE == nil {
				ptrToE = new(E)
			}
			if x.As(ptrToE) {
				return *ptrToE, true
			}
		}
		err = Unwrap(err)
	}
	var zero E
	return zero, false
}

cc: @Jorropo who actually wrote the second implementation of AsOf

Benchmarks

type MyError struct{}

func (me *MyError) Error() string {
	return "my error"
}

var sink *MyError

func AsOf[E error](err error) (E, bool) {
	e := new(E)
	ok := errors.As(err, e)
	return *e, ok
}

func BenchmarkAsOf(b *testing.B) {
	var err error = &MyError{}

	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		sink, _ = AsOf[*MyError](err)
	}
}

func AsOf2[E error](err error) (E, bool) {
	var ptrToE *E
	for err != nil {
		if e, ok := err.(E); ok {
			return e, true
		}
		if x, ok := err.(interface{ As(any) bool }); ok {
			if ptrToE == nil {
				ptrToE = new(E)
			}
			if x.As(ptrToE) {
				return *ptrToE, true
			}
		}
		err = errors.Unwrap(err)
	}
	var zero E
	return zero, false
}

func BenchmarkAsOf2(b *testing.B) {
	var err error = &MyError{}

	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		sink, _ = AsOf2[*MyError](err)
	}
}

Results:

First implementation:
goos: windows
goarch: amd64
pkg: test
cpu: Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
BenchmarkAsOf-16    	16641680	        70.14 ns/op	       8 B/op	       1 allocs/op
BenchmarkAsOf-16    	16340714	        70.77 ns/op	       8 B/op	       1 allocs/op
BenchmarkAsOf-16    	16931168	        71.64 ns/op	       8 B/op	       1 allocs/op
BenchmarkAsOf-16    	16693258	        71.01 ns/op	       8 B/op	       1 allocs/op
BenchmarkAsOf-16    	16718749	        70.24 ns/op	       8 B/op	       1 allocs/op
PASS
ok  	test	6.458s



Second implementation:
goos: windows
goarch: amd64
pkg: test
cpu: Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
BenchmarkAsOf2-16    	604845721	         1.940 ns/op	       0 B/op	       0 allocs/op
BenchmarkAsOf2-16    	618726052	         1.925 ns/op	       0 B/op	       0 allocs/op
BenchmarkAsOf2-16    	621167846	         1.928 ns/op	       0 B/op	       0 allocs/op
BenchmarkAsOf2-16    	608969202	         1.962 ns/op	       0 B/op	       0 allocs/op
BenchmarkAsOf2-16    	617731360	         1.905 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	test	7.138s
@gopherbot gopherbot added this to the Proposal milestone Nov 27, 2022
@Jorropo
Copy link
Member

Jorropo commented Nov 27, 2022 •
edited

I think it's important to point out that errors.As can be quite confusing to use, it has many ways it can panic at runtime all related to feeding a wrong type in the any argument: https://cs.opensource.google/go/go/+/refs/tags/go1.19.3:src/errors/wrap.go;l=79-90
This solution instead replace it with fully compile-time safe solution, by using the type checker and generic constraints.

@fzipp
Copy link
Contributor

fzipp commented Nov 27, 2022

The proposed name is strange. "as of" is a temporal preposition as in "as of January 2022".

@phenpessoa
Copy link
Author

@fzipp im open to suggestions for the name, as I’m not a fan of it either.

But the idea of using the Of suffix came from here: #48287

@fzipp
Copy link
Contributor

fzipp commented Nov 27, 2022

Yes, in my opinion, before adding generic variants of existing functions and types to the standard library, a general approach to this topic (#48287) should be decided first.

@seankhliao
Copy link
Member

Duplicate of #51945

@seankhliao seankhliao marked this as a duplicate of #51945 Nov 27, 2022
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Nov 27, 2022
@phenpessoa
Copy link
Author

@seankhliao the proposed implementation is not the same
Should we suggest this implementation on that issue?

@phenpessoa phenpessoa mentioned this issue Nov 27, 2022
Open
@Jorropo
Copy link
Member

Jorropo commented Nov 28, 2022 •
edited

@Pedro-Pessoa proposals are for API changes and some observable behaviour side effects.
My implementation and #51945 ones have identical API and behaviour this is then not concerned by the proposal process. (if #51945 is accepted I would be allowed to submit a CL with my implementation)

@golang golang locked and limited conversation to collaborators Nov 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
5 participants
@fzipp @phenpessoa @gopherbot @seankhliao @Jorropo