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: reflect: Add Type.Fields() iter.Seq[StructField] and Type.Methods() iter.Seq[Method] #66631

Open
earthboundkid opened this issue Apr 1, 2024 · 26 comments
Labels
Milestone

Comments

@earthboundkid
Copy link
Contributor

earthboundkid commented Apr 1, 2024

Proposal Details

reflect.Type.Fields() iter.Seq[StructField] and reflect.Type.Methods() iter.Seq[Method] would be more convenient than having to call i := range myType.NumFields() and myType.Field(i) etc.

You could also add iterators to reflect.Value as well but those are less obviously useful. Probably worth doing just for completeness though.

Edit: The proposal is now to return an iter.Seq2[int, StructField] so you can use .Field(i).

@gopherbot gopherbot added this to the Proposal milestone Apr 1, 2024
@thediveo
Copy link
Contributor

thediveo commented Apr 1, 2024

could this be a duplicate of #66626?

@adonovan
Copy link
Member

adonovan commented Apr 1, 2024

could this be a duplicate of #66626?

No, that issue relates to the go/types package, not reflect.

@thediveo
Copy link
Contributor

thediveo commented Apr 1, 2024

ah, overlooked this crucial detail, thank you!

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Apr 1, 2024
@earthboundkid
Copy link
Contributor Author

This was in fact inspired by #66626, but yes, it is for reflect, not types.

@adonovan
Copy link
Member

adonovan commented Apr 2, 2024

Alternatively, the method value could be the iterator as opposed to returning an iterator, like so:

package reflect

type Type interface {
	...
	// Fields is a go1.23 iterator over all the fields of a struct type (etc).
	//
	// Example: for ftype := range t.Fields { ... }
	Fields(yield func(ftype *Func) bool)
}

func (t *rtype) Fields(yield func(ftype *Func) bool) {
	for i := range t.NumFields() {
		if !yield(t.Field(i)) {
			break
		}
	}
}
// client code
var t reflect.Type = ...
for ftype := range t.Fields { ... }

@earthboundkid
Copy link
Contributor Author

Maybe. That keeps from introducing an import dependency we might not want, but it goes against the general conventions used elsewhere.

@earthboundkid
Copy link
Contributor Author

I used this in a project and the signature should be iter.Seq2[int, reflect.StructField] so that you can do rv.Field(i).Set(whatever) if the field matches some criteria. It's very convenient though and worth having.

@DeedleFake
Copy link

I'd like an iterator version of VisibleFields(), too. I had a need for that the other day, actually.

@earthboundkid
Copy link
Contributor Author

Isn't that just a filter on field.IsExported() or is there more to it than that? I did only want the exported fields for my thing, but I also needed a type and tag test for the field, so having VisibleFields() per se wouldn't have saved much coding.

@DeedleFake
Copy link

VisibleFields() gets the fields recursively into anonymous struct fields as well as unexported ones.

@earthboundkid
Copy link
Contributor Author

Got it. There's already a slice version of VisibleFields(), so I'm not sure how much the iterator version saves since you'll presumably cache the slice somehow if performance is a concern. I'm willing to be wrong though. There could be AllVisibleFields and VisibleFields could just call and collect that.

@myaaaaaaaaa
Copy link

myaaaaaaaaa commented Sep 10, 2024

It's possible to take Fields() out of the reflect package so that it can benefit more users:

package structs

func Fields(strukt any) iter.Seq2[string, any] {
	return func(yield func(string, any) bool) {
		strukt := reflect.ValueOf(strukt)
		isPointer := strukt.Kind() == reflect.Pointer
		if isPointer {
			strukt = strukt.Elem()
		}

		for i := range strukt.NumField() {
			fieldName := strukt.Type().Field(i).Name
			fieldVal := strukt.Field(i)
			if isPointer {
				fieldVal = fieldVal.Addr()
			}
			if !yield(fieldName, fieldVal.Interface()) {
				return
			}
		}
	}
}

Usage would be like so:

type vars struct {
	Editor string
	Lang   string
	Shell  string
}

func ExampleReadFields() {
	env := vars{
		Editor: "vim",
		Shell:  "/bin/dash",
	}

	for name, val := range structs.Fields(env) {
		name := strings.ToUpper(name)
		val := fmt.Sprint(val) // val is a string because env was passed by value
		os.Setenv(name, val)
	}

	fmt.Println(os.Environ())

	//Output:
	// [...    EDITOR=vim    SHELL=/bin/dash    ...]
}
func ExampleWriteFields() {
	var env vars

	for name, val := range structs.Fields(&env) {
		name := strings.ToUpper(name)
		*val.(*string) = os.Getenv(name) // val is a *string because &env was passed
	}

	fmt.Printf("%#v\n", env)

	//Output:
	// {Editor:"nano", Lang:"en_US.utf8", Shell:"/bin/bash"}
}
func ExampleStringKeys() {
	var env vars

	{
		envMap := maps.Collect(structs.Fields(&env))
		*envMap["Editor"].(*string) = "code"
		*envMap["Shell"].(*string) = "/bin/zsh"
	}

	fmt.Printf("%#v\n", env)

	//Output:
	// {Editor:"code", Lang:"", Shell:"/bin/zsh"}
}

This is also possible for Methods(), although I don't know anything about the real-world usage for that.

@earthboundkid
Copy link
Contributor Author

Package structs is about compiler magic fields. I'm not sure how it would help to move something out of reflect and into a different package if that package just imports reflect itself.

@myaaaaaaaaa
Copy link

myaaaaaaaaa commented Sep 10, 2024

Package structs is about compiler magic fields.

It seemed like the natural place to put it (akin to slices for slice functions and maps for map functions). Where else would you suggest?

I'm not sure how it would help to move something out of reflect and into a different package if that package just imports reflect itself.

Since there's no reference to reflect types in the signature, it allows users to avoid importing the reflect package, much like how encoding/json uses reflection but doesn't expose it.

@adonovan
Copy link
Member

This Fields function makes too many arbitrary choices: it returns field values or field addresses depending on whether the argument was a pointer; it accepts a pointer but panics if given a nil pointer; it discards type information; and it panics if any field is unexported. Each of these choices may be right for your project, but the result is not general enough to belong in the standard library. Users can always customize the loop below for their own needs:

	s := reflect.ValueOf(s)
	for i := range s.NumField() {
		use(s.Type().Field(i), s.Field(i))
	}

@myaaaaaaaaa
Copy link

it returns field values or field addresses depending on whether the argument was a pointer

This was intended to improve usability, and to make it more generalized and intuitive. Restricting it to pointer-to-structs would also work, although that seems more arbitrary to me.

it accepts a pointer but panics if given a nil pointer; and it panics if any field is unexported.

These were mistakes, not design choices. Sorry about that.

it discards type information

Can you elaborate on this? From a cursory glance, it seems like type information can be fully recovered from the field pointer.

for name, val := range structs.Fields(&env) {
	val := reflect.TypeOf(val)
	// ...
}

Users can always customize the loop below for their own needs

On the other hand, that would also seem to be an argument against the currently proposed Type.Fields().

@adonovan
Copy link
Member

From a cursory glance, it seems like type information can be fully recovered from the field pointer.

This returns the type of the field value, but not the reflect.StructField describing the field itself.

On the other hand, that would also seem to be an argument against the currently proposed Type.Fields().

Not at all. This proposal is for a modest and uncontroversial convenience method to enumerate each element of the Type.Field(i) sequence using go1.23 iterator conventions, nothing more.

@myaaaaaaaaa
Copy link

On the other hand, that would also seem to be an argument against the currently proposed Type.Fields().

Not at all. This proposal is for a modest and uncontroversial convenience method to enumerate each element of the Type.Field(i) sequence using go1.23 iterator conventions, nothing more.

In that case, should I file structs.Fields() as a separate proposal? Is there room in the standard library for both structs.Fields() and reflect.Type.Fields()?

@ianlancetaylor
Copy link
Member

I understand the appear of the name structs.Fields, but if we had such a function I do think it should be in the reflect package. In particular I think it would be something like

// Fields returns an iterator over the values of the fields of the struct v.
// It panics if v's kind is not [Struct].
func (v Value) Fields() iter.Seq[Value] {
    return func(func (Value) bool) {
        typ := v.Type()
        for i := range typ.NumFields() {
            if !yield(v.Field(i)) {
                return
            }
        }
    }
}

If we really want the field name also, it should perhaps be iter.Seq2[StructField, value].

I don't see any problem having both reflect.Type.Fields and reflect.Value.Fields. I do think they should be separate proposals.

@myaaaaaaaaa
Copy link

I understand the appear of the name structs.Fields, but if we had such a function I do think it should be in the reflect package. In particular I think it would be something like

func (v reflect.Value) Fields() iter.Seq[reflect.Value]

Sorry for making things confusing - structs.Fields(any) iter.Seq2[string, any] is a completely separate (and higher-level) function that iterates through structs as if they were map[string]anys, without needing to involve reflect types, but I mentioned it here just in case due to the overlap in functionality.

I don't see any problem having both reflect.Type.Fields and reflect.Value.Fields. I do think they should be separate proposals.

To clarify, does that mean structs.Fields (as described above) is also different enough to be a separate, third proposal?

@ianlancetaylor
Copy link
Member

To clarify, does that mean structs.Fields (as described above) is also different enough to be a separate, third proposal?

Yes. I don't think they really have anything to do with each other.

@dsnet
Copy link
Member

dsnet commented Sep 20, 2024

As a data point, I'm updating a bunch of Go code to use range-over-int, and I'm discovering many cases of:

for i := range t.NumField() {
    f := t.Field(i)
    ...
}

In fact, in the particular codebase I'm working on, that's the most common reason (~95%) to ever call NumField and Field is to iterate over all the fields.

@adonovan
Copy link
Member

In fact, in the particular codebase I'm working on, that's the most common reason (~95%) to ever call NumField and Field is to iterate over all the fields.

Unfortunately all your pleasing uses of range t.NumField() will go away when in go1.24 you'll be able to say just for f := range t.Fields() { ... }. ;-)

@dsnet
Copy link
Member

dsnet commented Sep 20, 2024

Such is the life of someone who cares about modern Go code 😫. I think I held the record for most Go code commits in a single day before I left Google.

@andig
Copy link
Contributor

andig commented Jan 11, 2025

I don't find t.Fields() in https://github.com/golang/go/blob/master/src/reflect/type.go?

@earthboundkid
Copy link
Contributor Author

This proposal hasn't been evaluated yet. #66626 will be in Go 1.24, however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

9 participants