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

proposal: spec: add builtin func 'is[T any](any) bool' #65846

Closed
2 of 4 tasks
adonovan opened this issue Feb 21, 2024 · 28 comments
Closed
2 of 4 tasks

proposal: spec: add builtin func 'is[T any](any) bool' #65846

adonovan opened this issue Feb 21, 2024 · 28 comments
Labels
generics Issue is related to generics LanguageChange Proposal
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Feb 21, 2024

Go Programming Experience

Experienced

Other Languages Experience

many

Related Idea

  • Has this idea, or one like it, been proposed before?
  • Does this affect error handling?
  • Is this about generics?
  • Is this change backward compatible? Breaking the Go 1 compatibility guarantee is a large cost and requires a large benefit

Has this idea, or one like it, been proposed before?

Not to my knowledge.

Does this affect error handling?

No.

Is this about generics?

Indirectly.

Proposal

I propose that we add an intrinsic is function, equivalent to this Go declaration:

// Is[T] reports whether the type assertion x.(T) would succeed.
func is[T any](x any) bool {
     _, ok := x.(T)
    return ok
}

This operator makes it possible to test the type of an interface without breaking out of expression mode, which is especially helpful when writing code that makes heavy use of sum types, such as syntax trees, types, XML documents, and so on.

if len(calleeDecl.Body.List) == 1 &&
                is[*ast.ReturnStmt](calleeDecl.Body.List[0]) &&
                len(calleeDecl.Body.List[0].(*ast.ReturnStmt).Results) > 0 { // not a bare return

Of course, one can write this function in Go or import it from a package, but like min and max it is ubiquitous and fundamental, and warrants a short name.

Language Spec Changes

The universe scope defines an is function, whose declaration is equivalent to the Go code shown above.

Informal Change

The is operator performs a test of an interface's dynamic type.

Is this change backward compatible?

Yes.

Orthogonality: How does this change interact or overlap with existing features?

Very orthogonal.

Would this change make Go easier or harder to learn, and why?

No net change. Harder, because one more thing to learn; easier, because less frustrating breaking out of expression mode.

Cost Description

No response

Changes to Go ToolChain

local changes to cmd/compile, go/types, a few others

Performance Costs

none

Prototype

See above.

@adonovan adonovan added LanguageChange v2 A language change or incompatible library change Proposal labels Feb 21, 2024
@gopherbot gopherbot added this to the Proposal milestone Feb 21, 2024
@jimmyfrasche
Copy link
Member

A more general builtin, and one that could not be written in the language today, would take anything with a 1- or 2-value form and return the second value. For lack of a better name I'll call it snd for second. With that you could write snd(v.(T)) or snd(m[k]). I have felt the need for this far more for maps, personally. That would also suggest a both builtin that forces the 2-value return. I have wanted both of those before, though mostly snd.

@ianlancetaylor ianlancetaylor added the generics Issue is related to generics label Feb 21, 2024
@adonovan
Copy link
Member Author

adonovan commented Feb 21, 2024

A more general builtin, and one that could not be written in the language today, would take anything with a 1- or 2-value form and return the second value. For lack of a better name I'll call it snd for second. With that you could write snd(v.(T)) or snd(m[k]). I have felt the need for this far more for maps, personally. That would also suggest a both builtin that forces the 2-value return. I have wanted both of those before, though mostly snd.

That is more general, but less clear:

  • snd(m[k]) would ask whether k is present in a map. You can write this today as m[k] (if all elements are true), or m[k] != nil or m[k] != 0 if the map contains only nonzero values, which is the common case. In the rare case that you want to distinguish absence from zero, it would be clearer to declare contains(m, k) as a function or use the explicit ,ok form.
  • snd(<-ch) would tell you whether you received a value from the channel, but would throw that value away. I can't think of a time I've needed that operation, but the fact is that concurrency operations are far less common than ordinary computation, and because of the side effects they essentially never appear in the middle of an expression context where using a statement would be onerous.

So it doesn't seem to me to be the right axis for generalization.

@jimmyfrasche
Copy link
Member

I have written plenty of code that wants to know if a value is in a map that must distinguish between zero and absent. Far more often than it was okay to rely on zero and absent being the same. I've wanted is maybe 3 or 4 times. I've wanted return both(m[k]) much more often and that's pretty rare.

Agreed that snd(<-ch) is fairly useless.

@DeedleFake
Copy link

I would like it to be possible to expand the optional-second-value expressions in certain circumstances. For example,

func Example[T any](v T) T { return v }
func Example2[T any](v T, ok bool) bool { return ok }

Example(m[k]) // Currently legal.
Example2(m[k]) // Not currently legal.

With that available, it would be extremely easy to just write a snd() implementation in those cases where it's useful without it being pre-declared and thus encouraging things like snd(<-c).

@adonovan
Copy link
Member Author

With that available, it would be extremely easy to just write a snd() implementation in those cases where it's useful without it being pre-declared and thus encouraging things like snd(<-c).

We're losing sight of the point. It's already possible to write an is implementation in those cases where it is useful, just like it is for max and min. The point of this issue is that there are so many such cases that it justifies being a buillt-in.

Evidence: grep for if _, ok := .*[.]\(.*\); ok on your favorite repo.

@gophun
Copy link

gophun commented Feb 21, 2024

Evidence: grep for if _, ok := .[.](.); ok on your favorite repo.

However, this grep alone does not indicate whether these occurrences would noticeably benefit from being usable in an expression. The difference between using if is[T](x) { and if _, ok := x.(T); ok { by itself is so minor that it doesn't even save a line.

@timothy-king
Copy link
Contributor

The difference between using if isT { and if _, ok := x.(T); ok { by itself is so minor that it doesn't even save a line.

is[T] composes a bit nicer if one needs to do multiple matches, e.g. x is a T and x.F is a U and x.G is a V if x, ok := x.(T); ok && is[U](x.F) && is[V](x.G) {}. This starts to save lines.

But the main question is whether the new builtin improves the readability of the code more than the cost of adding a new builtin into the language.

is[T] would help the readability for x/tools but YMMV.

@zephyrtronium
Copy link
Contributor

A thing I frequently see, especially with people first coming to Go from dynamically typed languages, is overuse of []any and map[string]any especially when working with JSON. I worry that having this is builtin would encourage trudging through that approach rather than defining types.

And personally, I don't find that there are many cases calling for is. Running a grep for just _, ok on all code I have cloned for work finds zero uses for type assertions, only map lookups plus some channel receives in tests, out of about 168k lines of Go.

@apparentlymart
Copy link

apparentlymart commented Feb 21, 2024

As suggested in an earlier comment, I did a (very unscientific) survey of the codebase I spend my dayjob maintaining, for examples that might benefit from this proposal.

This codebase seems to have a pretty even split between _, ok being used with type assertions vs. being used for testing for the presence of map keys, along with a small number of functions specific to this codebase that follow the (result, bool) convention.

During this survey I found three main situations.

  • Most common: Detecting what type an interface isn't, rather than what it is:

        if _, ok := b.(*Example); !ok {
            return fmt.Errorf(...)
        }

    I think this is most common because this sort of negative type assertion doesn't generate any new information about what dynamic type the interface does have; the first result is always useless.

    It seems that most (possibly all) of these examples would slightly benefit in conciseness from this proposal:

        if !is[*Example](b) {
            return fmt.Errorf(...)
        }

    ...although I have mixed feelings about whether this is more or less readable to someone who isn't already familiar with this is function, since the is identifier seems to get lost in a swamp of punctuation.

  • A lot less common: Detecting what type an interface (most often an error) has, without any care about the value.

        if _, ok := err.(SpecialError); ok {
            // Different handling for this special error type
        }

    For errors in particular it seems relatively common for the dynamic type to be the most important information, with the data inside that value being less important. This isn't true in all cases of course -- sometimes we're treating a particular error type as special specifically so that we can rely on some fields or methods unique to that error type -- but making the decision based on type alone seems somewhat common in our codebase.

    These examples could also be trivially replaced by a call to is:

        if is[SpecialError](err) {
            // Different handling for this special error type
        }
  • Dishonorable mention: a series of tests that could have and should have been a type switch.

        if _, ok := k.(Example1); ok {
            return Example1Result
        }
        if _, ok := k.(Example2); ok {
            return Example2Result
        }
        return OtherResult

    Thankfully I found few enough examples of this that I can count them on one hand. Of course I would rewrite these to be type switches instead of adopting the is builtin for them.

        switch k.(type) {
        case Example1:
            return Example1Result
        case Example2:
            return Example2Result
        default:
            return OtherResult
        }

This survey contradicted my initial (incorrect) hunch that there wouldn't be many examples of testing the type without then using the value. I expect we'd have plenty of opportunities to use is if it were accepted as a builtin.

However, I feel ambivalent because of the highly subjective concern I mentioned above: that the very short identifier is gets visually lost in a lot of punctuation that seems to make this harder to read and understand than I had initially hoped. The longer forms we're using today are less concise, but the meaning "feels" clearer to me.

I expect this concern would reduce if I were exposed to more examples of this in real code, but I'd worry about folks who are new to Go -- who might not have yet learned to read identifer[Something] as Go's generics syntax, due to it being different to most other languages -- being unsure how to mentally parse is[*Thingy](something) as "Is *Thingy the dynamic type of something?"

Perhaps choosing a longer name would allow being more specific about what exactly this function means and thus better prime someone to guess what the following characters mean. I don't want to bikeshed about it though; I'm curious to see if others share this concern or if it's just me.

Given all of the above, I'm cautiously in favor.

@apparentlymart
Copy link

It occurs to me that predeclared builtins are allowed more flexibility in their signatures, including taking types directly as arguments within the parentheses as we see with e.g. make([]Foo, 0, 5).

Therefore this proposal could presumably be spelled is(v, Type) instead, which might be more readable both due to having less punctuation and also following the typical English term order for sentences: "Is v a Type?" "Is err a ParseError?"

That of course does come at the expense of orthogonality, and at the expense of a potential opportunity to introduce a newcomer to the syntax for qualifying a generic function when type inference isn't possible.

@bjorndm
Copy link

bjorndm commented Feb 22, 2024

I'd like to note that this may conflict with the package https://github.com/matryer/is
is is an extremely common identifier so making it built-in now will make it hard to use. Some different name may be needed.

@DeedleFake
Copy link

I'd like to note that this may conflict with the package https://github.com/matryer/is is is an extremely common identifier so making it built-in now will make it hard to use. Some different name may be needed.

That shouldn't be a problem any more than adding min() and max() were. Existing code will continue to work with no difference and anyone who wants to use it can just change the name of the import.

@seankhliao seankhliao changed the title proposal: predeclared 'is[T any](any) bool' proposal: Go 2: predeclared 'is[T any](any) bool' Feb 23, 2024
@ianlancetaylor ianlancetaylor removed the v2 A language change or incompatible library change label Mar 6, 2024
@adonovan
Copy link
Member Author

adonovan commented Mar 9, 2024

I wrote an analyzer to detect functions equivalent to is[T] or some instantiation of it, and ran it across a sample (min imports = 10) of the corpus of the Go module mirror. Out of 21477 modules, 548 (2.5%) define at least one function equivalent to is for some T. The query found a total of 1504 such functions. I have included a random sample of 100 of them below.

This measure undercounts situations in which is[T] would be useful. For example, it does not attempt to tally cases in which two statements:

    cond := false
    if _, ok := x.(X); ok {
        if _, ok := y.(Y); ok {
            cond = true
        }
    }
    f(cond)

could be replaced by one expression:

    f(is[X](x) && is[Y](y))

As @seankhliao points out, approximately half are concerned with errors, and I have divided the list below into two halves, non-error-related and error-related.

NON-ERROR RELATED:

https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/v12@v12.0.1/parquet/schema/logical_types.go#L1055: is[schema.NullLogicalType]
https://go-mod-viewer.appspot.com/github.com/leanovate/gopter@v0.2.9/arbitrary/gen_for_kind_test.go#L107: is[float32]
https://go-mod-viewer.appspot.com/github.com/galdor/go-ejson@v0.0.0-20231201100034-d335379f26b0/values.go#L38: is[map[string]interface{}]
https://go-mod-viewer.appspot.com/github.com/onflow/atree@v0.6.0/cmd/main/main.go#L84: is[main.testTypeInfo]
https://go-mod-viewer.appspot.com/github.com/unidoc/unipdf/v3@v3.55.0/contentstream/contentstream.go#L35: is[*model.PdfColorspaceSpecialPattern]
https://go-mod-viewer.appspot.com/github.com/df-mc/dragonfly@v0.9.13/server/block/terracotta.go#L13: is[block.DeadBush]
https://go-mod-viewer.appspot.com/github.com/nuvolaris/goja@v0.0.0-20230825100449-967811910c6d/value.go#L448: is[goja.valueNull]
https://go-mod-viewer.appspot.com/github.com/nicocha30/gvisor-ligolo@v0.0.0-20230726075806-989fa2c0a413/pkg/sentry/devices/tundev/tundev.go#L189: is[*netstack.Stack]
https://go-mod-viewer.appspot.com/github.com/argoproj/argo-cd@v1.8.7/util/lua/lua.go#L277: is[[]interface{}]
https://go-mod-viewer.appspot.com/github.com/llir/llvm@v0.3.6/ir/types/types.go#L68: is[*types.IntType]
https://go-mod-viewer.appspot.com/github.com/google/go-safeweb@v0.0.0-20231219055052-64d8cfc90fbb/safehttp/plugins/csp/strict.go#L96: is[internalunsafecsp.DisableStrict]
https://go-mod-viewer.appspot.com/github.com/influxdata/influxql@v1.1.0/ast.go#L5450: is[influxql.Literal]
https://go-mod-viewer.appspot.com/github.com/MontFerret/ferret@v0.18.0/pkg/runtime/expressions/literals/none.go#L18: is[*literals.noneLiteral]
https://go-mod-viewer.appspot.com/gotest.tools/gotestsum@v1.11.0/cmd/main.go#L464: is[cmd.exitCoder]
https://go-mod-viewer.appspot.com/github.com/unidoc/unidoc@v2.2.0+incompatible/pdf/contentstream/processor.go#L351: is[*model.PdfColorspaceSpecialPattern]
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/v15@v15.0.1/parquet/schema/logical_types.go#L211: is[schema.StringLogicalType]
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/v14@v14.0.2/arrow/compute/exec/kernel.go#L236: is[exec.primitiveMatcher]
https://go-mod-viewer.appspot.com/github.com/Psiphon-Labs/tls-tris@v0.0.0-20230824155421-58bf6d336a9a/handshake_messages.go#L2375: is[*tls.serverHelloDoneMsg]
https://go-mod-viewer.appspot.com/github.com/facebookincubator/go-belt@v0.0.0-20230703220935-39cd348f1a38/tool/experimental/tracer/noop_span.go#L89: is[*tracer.NoopSpan]
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/v12@v12.0.1/parquet/schema/logical_types.go#L445: is[schema.DateLogicalType]
https://go-mod-viewer.appspot.com/github.com/klaytn/klaytn@v1.12.1/blockchain/types/transaction_signing.go#L529: is[types.HomesteadSigner]
https://go-mod-viewer.appspot.com/github.com/leanovate/gopter@v0.2.9/arbitrary/gen_for_kind_test.go#L101: is[uint64]
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/v12@v12.0.1/parquet/schema/logical_types.go#L1013: is[schema.IntervalLogicalType]
https://go-mod-viewer.appspot.com/github.com/moleculer-go/moleculer@v0.3.3/service/service.go#L990: is[func(moleculer.Payload)]
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/v15@v15.0.1/arrow/compute/exec/kernel.go#L205: is[exec.largeBinaryLikeMatcher]
https://go-mod-viewer.appspot.com/github.com/leanovate/gopter@v0.2.9/arbitrary/gen_for_kind_test.go#L71: is[int16]
https://go-mod-viewer.appspot.com/github.com/ydb-platform/ydb-go-sdk/v3@v3.57.0/internal/types/types.go#L307: is[types.EmptyDict]
https://go-mod-viewer.appspot.com/github.com/dolthub/go-mysql-server@v0.18.0/sql/types/linestring.go#L79: is[types.LineStringType]
https://go-mod-viewer.appspot.com/sigs.k8s.io/kubebuilder/v3@v3.14.0/pkg/cli/webhook.go#L49: is[plugin.CreateWebhook]
https://go-mod-viewer.appspot.com/github.com/kubevela/workflow@v0.6.0/pkg/cue/model/sets/operation.go#L393: is[*ast.Ellipsis]
https://go-mod-viewer.appspot.com/github.com/BTBurke/caddy-jwt@v3.7.1+incompatible/key_utils.go#L31: is[*rsa.PublicKey]
https://go-mod-viewer.appspot.com/github.com/decred/politeia@v1.4.0/politeiawww/legacy/user/cockroachdb/cockroachdb_test.go#L31: is[[]byte]
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/v14@v14.0.2/parquet/schema/logical_types.go#L1062: is[schema.IntervalLogicalType]
https://go-mod-viewer.appspot.com/github.com/decred/politeia@v1.4.0/politeiawww/legacy/user/mysql/mysql_test.go#L30: is[[]byte]
https://go-mod-viewer.appspot.com/github.com/hashicorp/hcl/v2@v2.20.0/expr_call.go#L22: is[hcl.exprCall]
https://go-mod-viewer.appspot.com/github.com/dolthub/go-mysql-server@v0.18.0/sql/types/polygon.go#L84: is[types.PolygonType]
https://go-mod-viewer.appspot.com/github.com/sdboyer/gps@v0.16.3/constraints.go#L134: is[gps.anyConstraint]
https://go-mod-viewer.appspot.com/github.com/tomwright/dasel@v1.27.3/error.go#L72: is[*dasel.UnsupportedTypeForSelector]
https://go-mod-viewer.appspot.com/gonum.org/v1/gonum@v0.14.0/mat/list_test.go#L187: is[*mat.VecDense]
https://go-mod-viewer.appspot.com/github.com/mantzas/incata@v0.3.0/writer/sql_writer_test.go#L26: is[time.Time]
https://go-mod-viewer.appspot.com/github.com/asynkron/protoactor-go@v0.0.0-20240308120642-ef91a6abee75/eventstream/eventstream_example_subscribe_test.go#L17: is[string]
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/v7@v7.0.1/parquet/schema/logical_types.go#L1013: is[schema.IntervalLogicalType]
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/v12@v12.0.1/parquet/schema/logical_types.go#L1092: is[schema.NoLogicalType]
https://go-mod-viewer.appspot.com/go.chromium.org/luci@v0.0.0-20240309015107-7cdc2e660f33/common/tsmon/store/nil.go#L32: is[store.nilStore]
https://go-mod-viewer.appspot.com/github.com/tomwright/dasel@v1.27.3/error.go#L55: is[*dasel.UnsupportedSelector]
https://go-mod-viewer.appspot.com/github.com/gochain/gochain/v3@v3.4.9/core/types/transaction_signing.go#L189: is[types.FrontierSigner]
https://go-mod-viewer.appspot.com/k8s.io/kube-openapi@v0.0.0-20240228011516-70dd3763d340/pkg/internal/third_party/go-json-experiment/json/decode.go#L264: is[*bytes.Buffer]

ERROR-RELATED:

https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/error.go#L51: is[models.ErrUserOwnPackages]
https://go-mod-viewer.appspot.com/github.com/xelaj/errs@v0.0.0-20200831133608-d1c11863e019/defs.go#L61: is[*errs.NotFoundError]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/db/name.go#L49: is[db.ErrNamePatternNotAllowed]
https://go-mod-viewer.appspot.com/github.com/mendersoftware/go-lib-micro@v0.0.0-20240304135804-e8e39c59b148/rest_utils/api_error.go#L31: is[*rest_utils.ApiError]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/auth/oauth2.go#L593: is[auth.ErrOAuthClientIDInvalid]
https://go-mod-viewer.appspot.com/github.com/cyverse/go-irodsclient@v0.13.2/irods/types/error.go#L58: is[*types.ConnectionError]
https://go-mod-viewer.appspot.com/github.com/govau/cf-common@v0.0.7/env/env.go#L140: is[*env.varNotFoundError]
https://go-mod-viewer.appspot.com/gopkg.in/hedzr/errors.v3@v3.3.1/tool.go#L76: is[interface{Is(error) bool}]
https://go-mod-viewer.appspot.com/github.com/cmd-stream/base-go@v0.0.0-20230813145615-dd6ac24c16f5/client/client.go#L345: is[net.Error]
https://go-mod-viewer.appspot.com/github.com/lulzWill/go-agent@v2.1.2+incompatible/internal/utilization/provider.go#L24: is[utilization.validationError]
https://go-mod-viewer.appspot.com/github.com/lbryio/lbcd@v0.22.119/blockchain/indexers/common.go#L87: is[indexers.errDeserialize]
https://go-mod-viewer.appspot.com/github.com/hashicorp/terraform-plugin-sdk@v1.17.2/internal/registry/errors.go#L52: is[*registry.ServiceUnreachableError]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/user/error.go#L107: is[user.ErrUserIsNotLocal]
https://go-mod-viewer.appspot.com/gitlab.com/evatix-go/core@v1.3.55/iserror/ExitError.go#L5: is[*exec.ExitError]
https://go-mod-viewer.appspot.com/github.com/alloyzeus/go-azfl@v0.0.0-20231220071816-9740126a2d07/errors/entity.go#L14: is[errors.EntityError]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/repo/release.go#L31: is[repo.ErrReleaseAlreadyExist]
https://go-mod-viewer.appspot.com/github.com/dashpay/godash@v0.0.0-20160726055534-e038a21e0e3d/blockchain/indexers/common.go#L79: is[indexers.errDeserialize]
https://go-mod-viewer.appspot.com/github.com/lestrrat-go/jwx/v2@v2.0.21/jwt/validate.go#L187: is[*jwt.invalidAudienceError]
https://go-mod-viewer.appspot.com/github.com/hashicorp/terraform-plugin-sdk@v1.17.2/internal/registry/errors.go#L21: is[*registry.errModuleNotFound]
https://go-mod-viewer.appspot.com/github.com/palcoin-project/palcd@v1.0.0/blockchain/indexers/common.go#L87: is[indexers.errDeserialize]
https://go-mod-viewer.appspot.com/github.com/gitbundle/modules@v0.0.0-20231025071548-85b91c5c3b01/lfs/content_store.go#L34: is[lfs.ErrRangeNotSatisfiable]
https://go-mod-viewer.appspot.com/github.com/cavaliergopher/grab/v3@v3.0.1/error.go#L39: is[grab.StatusCodeError]
https://go-mod-viewer.appspot.com/github.com/jurado-dev/errors@v1.2.10/error_types.go#L42: is[*errors.BadRequest]
https://go-mod-viewer.appspot.com/github.com/BlockABC/godash@v0.0.0-20191112120524-f4aa3a32c566/blockchain/chainio.go#L73: is[blockchain.errDeserialize]
https://go-mod-viewer.appspot.com/github.com/blend/go-sdk@v1.20220411.3/envoyutil/xfcc_errors.go#L44: is[*envoyutil.XFCCExtractionError]
https://go-mod-viewer.appspot.com/github.com/btcsuite/btcd@v0.24.0/blockchain/chainio.go#L106: is[blockchain.errDeserialize]
https://go-mod-viewer.appspot.com/github.com/cloudwego/kitex@v0.9.0/pkg/utils/func_test.go#L36: is[*reflect.ValueError]
https://go-mod-viewer.appspot.com/k8s.io/perf-tests/clusterloader2@v0.0.0-20240304094227-64bdb12da87e/pkg/util/util.go#L41: is[*util.ErrKeyNotFound]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/asymkey/error.go#L78: is[asymkey.ErrKeyNameAlreadyUsed]
https://go-mod-viewer.appspot.com/github.com/jurado-dev/errors@v1.2.10/error_types.go#L72: is[*errors.NotFound]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/asymkey/error.go#L36: is[asymkey.ErrKeyNotExist]
https://go-mod-viewer.appspot.com/github.com/whyrusleeping/gx@v0.14.3/gxutil/get.go#L17: is[gxutil.ErrAlreadyInstalled]
https://go-mod-viewer.appspot.com/go.uber.org/cadence@v1.2.9/error.go#L92: is[*internal.PanicError]
https://go-mod-viewer.appspot.com/github.com/cenkalti/backoff/v4@v4.2.1/retry.go#L133: is[*backoff.PermanentError]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/services/asymkey/sign.go#L80: is[*asymkey.ErrWontSign]
https://go-mod-viewer.appspot.com/gitee.com/hongliu9527/go-tools@v0.0.8/errors/gerror/gerror_api_stack.go#L77: is[gerror.IStack]
https://go-mod-viewer.appspot.com/github.com/spotmaxtech/k8s-apimachinery-v0260@v0.0.1/pkg/api/meta/errors.go#L122: is[*meta.NoKindMatchError]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/issues/reaction.go#L47: is[issues.ErrReactionAlreadyExist]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/auth/webauthn.go#L38: is[auth.ErrWebAuthnCredentialNotExist]
https://go-mod-viewer.appspot.com/github.com/OrigamiWang/msd/micro@v0.0.0-20240229032328-b62246268db9/model/errx/error.go#L34: is[errx.ErrX]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/project/project.go#L56: is[project.ErrProjectNotExist]
https://go-mod-viewer.appspot.com/github.com/ethereumproject/go-ethereum@v5.5.2+incompatible/core/error.go#L65: is[*core.UncleErr]
https://go-mod-viewer.appspot.com/github.com/verrazzano/verrazzano@v1.7.0/pkg/controller/errors/errors.go#L52: is[spi.RetryableError]
https://go-mod-viewer.appspot.com/github.com/vicanso/hes@v0.7.0/http_errors.go#L284: is[*hes.Error]
https://go-mod-viewer.appspot.com/github.com/daixiang0/gci@v0.13.0/pkg/section/errors.go#L104: is[section.FileParsingError]
https://go-mod-viewer.appspot.com/github.com/interconnectedcloud/qdr-operator@v0.0.0-20210826174505-576d2b33dac7/test/e2e/framework/retry_util.go#L31: is[*framework.RetryError]
https://go-mod-viewer.appspot.com/github.com/keybase/client/go@v0.0.0-20240309051027-028f7c731f8b/kbfs/libfuse/dir.go#L967: is[idutil.NoSuchNameError]
https://go-mod-viewer.appspot.com/github.com/decred/dcrd/blockchain@v1.2.1/chainio_test.go#L71: is[blockchain.errNotInMainChain]
https://go-mod-viewer.appspot.com/gopkg.in/hedzr/errors.v3@v3.3.1/tool.go#L28: is[errors.causer]
https://go-mod-viewer.appspot.com/code.gitea.io/gitea@v1.21.7/models/user/email_address.go#L53: is[user.ErrEmailInvalid]

@seankhliao
Copy link
Member

it looks like over half of those deal with errors, which could be errors.As(err, &T{})

@thepudds
Copy link
Contributor

thepudds commented Mar 9, 2024

@adonovan, if this was a function in the standard library instead of a builtin, would you view that as acceptable, and any brief thoughts on how you would spell it if so? (Sorry if you already mentioned this).

it looks like over half of those deal with errors, which could be errors.As(err, &T{})

A quick side note is I think @Merovius has observed that if errors.As had been designed after generics were available, it might have been errors.As[T error](err error) (T, bool) instead, which is arguably more ergonomic.

@fzipp
Copy link
Contributor

fzipp commented Mar 9, 2024

A quick side note is I think @Merovius has observed that if errors.As had been designed after generics were available, it might have been errors.As[T error](err error) (T, bool) instead, which is arguably more ergonomic.

No, generics were considered at that time and deemed less ergonomic: #29934 (comment)

@zigo101
Copy link

zigo101 commented Mar 9, 2024

Now, none builtin functions use the custom generic syntax. So maybe it is not a bad idea to support this function in std instead in my opinion.

BTW, @rsc's reply on the generic form of Is and As: #29934 (comment) (Ah, someone posted this ahead of me. :))

@adonovan
Copy link
Member Author

adonovan commented Mar 9, 2024

Update: Thanks for all the comments. I fixed a couple bugs in the analyzer (https://go.dev/cl/570315) and re-ran it, and updated the list, now partitioned into error and non-error-related cases (roughly 50% each).

I agree that a generic errors.As would have been nice, but perhaps that ship has sailed.

if this was a function in the standard library .. how you would spell it if so?

No package springs to mind; it's essentially an operator in the language. The closest analogue is min and max, but they obviously belong in the "math" package; there's no obvious counterpart here.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/570315 mentions this issue: go/analysis/passes/is: find is[T] functions

@jimmyfrasche
Copy link
Member

I am curious how those numbers compare to _, ok := m[k]. Are there docs somewhere about how to run these kinds of analyses and how to get a snapshot of the module proxy or is this something that requires special access?

@Merovius
Copy link
Contributor

Merovius commented Mar 9, 2024

Would this change make Go easier or harder to learn, and why?
No net change. Harder, because one more thing to learn; easier, because less frustrating breaking out of expression mode.

This feels like a kind of disingenuous answer. "Less frustrating breaking out of expression mode" is not a learnability benefit, it's a convenience. I think this is a pretty clear net negative (albeit not a huge one) when it comes to learnability.

Personally, I feel rather strongly that this proposal represents an inflationary attitude towards predeclared functions. For most of Go's life, we had a very limited number of predeclared identifiers and that was always advertised as a big benefit. And we only used predeclared functions for things that we could not implement in the language.

That changed with min/max, which where only made predeclared to avid the import. Now their existence is taken as evidence that we might as well add more.

Not to go all old-man-yelling-at-cloud, but I don't like it. I disliked adding min/max, I dislike is and I will probably dislike the next proposed predeclared function that creates a minor convenience.

@bjorndm
Copy link

bjorndm commented Mar 10, 2024

For me it is more the problem that there are already several different kinds of is, for equality, error handling or unit testing, declared in different packages, so adding one more seem not particularly helpful, at least not with that name.

The min and max are somewhat different as they were likely used only for minimum and maximum.

For this proposal something like typeIs seems better to me.

@zigo101
Copy link

zigo101 commented Mar 10, 2024

If min and max are not builtin, then

  1. their performance would be not good enough, because a slice is needed to be created at run time.
  2. people are not willing to use them because a package needs to be imported but provided functionality is so tiny.

@rsc rsc changed the title proposal: Go 2: predeclared 'is[T any](any) bool' proposal: spec: add predeclared 'is[T any](any) bool' Mar 15, 2024
@rsc rsc changed the title proposal: spec: add predeclared 'is[T any](any) bool' proposal: spec: add builtin func 'is[T any](any) bool' Mar 15, 2024
@rsc
Copy link
Contributor

rsc commented Mar 15, 2024

Adding to the ordinary proposal minutes, but I don't think this holds its weight. The vast majority of the time you care about the underlying type of something, you care because you want to use it. The x, ok := v.(T) form gives you the value to use. I can see how having is would be useful in complex conditionals, inside which you would re-type-assert to get the value. That may come up often in the kinds of AST code you write, but for me it almost never comes up. It's necessarily a feature of localized value.

To me at least, this doesn't seem worthwhile enough. The language already provides two forms - type assertions and type switches - so the bar for adding a third equivalent form seems quite high to me, and I don't think we have evidence here of meeting that bar.

@rsc
Copy link
Contributor

rsc commented Mar 15, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@it512
Copy link

it512 commented Mar 15, 2024

func is (t any, aType ... any) (any,bool) {
// if t.(type) in aType , return first aType and true
// else return nil ,false
}

@rsc
Copy link
Contributor

rsc commented Mar 27, 2024

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 4, 2024

No change in consensus, so declined.
— rsc for the proposal review group

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generics Issue is related to generics LanguageChange Proposal
Projects
Status: Declined
Development

No branches or pull requests