errors: Is should not panic

[cuiweixie]

What version of Go are you using (go version)?

$ go version
go version devel go1.20-18e01e9436 Sat Aug 27 15:34:47 2022 +0800 darwin/arm64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
darwin/arm64

What did you do?

package main

import "errors"

type Error struct {
	err [1]interface{}
}

func (e Error) Error() string {
	return ""
}

func main() {
	a := Error{
		err:[1]interface{}{[]string{"foo"}},
	}
	is := errors.Is(a, a)
	println(is)
}

What did you expect to see?

exit normaly

What did you see instead?

panic

panic: runtime error: comparing uncomparable type []string

[gopherbot]

Change https://go.dev/cl/426174 mentions this issue: errors: Is should check comparable of value not type

[cuiweixie]

similiar #54708

[cuonglm]

Panic seems to be the right behavior, the errors.Is doc states (emphasize mine):

An error is considered to match a target if it is equal to that target or if it implements a method Is(error) bool such that Is(target) returns true.

[cuiweixie]

in my opinion, it's a better choice to avoid panic here. the Value.Comparable api is invented to deal with situation like this.
#46746 (comment)
@ianlancetaylor

[hopehook]

Some of my thoughts

Why the old version panic:
When the panic message says: panic: runtime error: comparing uncomparable types []string.
That means the problem is that we should find a way to make it comparable, but we can't. So, panic now.

This CL's problem:
If you use Value.Comparable instead, you'll get into the bad situation of errors.Is(a, a) == false because comparing the uncomparable type []string. However the result should still be true because it's actually the same error a and a.

What to do?
To avoid the above situation, we need a mechanism like DeepEqual to make judgments in errors.Is(a, a), not instead of a == a(err == target).

[cuiweixie]

adding a Is method to Error can make it return true.

package main

import "errors"

type Error struct {
	err [1]interface{}
}

func (e Error) Error() string {
	return ""
}

func (e Error) Is(err error) bool {
	_, ok := err.(Error)
	return ok
}

func main() {
   var err = Error{err:[1]interface{}{[]int{}}}
   ok := errors.Is(err, err)
   println(ok)
}

deepequal is too heavy.

[hopehook]

func (e Error) Is(err error) bool {
_, ok := err.(Error)
return ok
}

My main concern is how errors.Is proves that a is a itself, which your example doesn't address.
From my limited knowledge, currently only DeepEqual can do it.

[gopherbot]

Change https://go.dev/cl/554456 mentions this issue: errors: Is function do not panic for uncomparable type

[qiulaidongfeng]

Some of my thoughts

Why the old version panic: When the panic message says: panic: runtime error: comparing uncomparable types []string. That means the problem is that we should find a way to make it comparable, but we can't. So, panic now.

This CL's problem: If you use Value.Comparable instead, you'll get into the bad situation of errors.Is(a, a) == false because comparing the uncomparable type []string. However the result should still be true because it's actually the same error a and a.

What to do? To avoid the above situation, we need a mechanism like DeepEqual to make judgments in errors.Is(a, a), not instead of a == a(err == target).

See https://github.com/golang/go/blob/master/src/reflect/deepequal.go#L154C1-L159C15 , for uncomparable types, a!=a, there are precedents.

[qiulaidongfeng]

I think panic should be used in situations where the code should not continue to execute, such as unlocking an unlocked mutex.
That is clearly not the case with this issue.
It's not hard to fix the right behavior here, but it's hard not to affect performance.