reflect: MakeStruct

[sbinet]

Feature request for MakeStruct, to go along with the newly-arrived MakeFunc ( issue #1765
).

It would be greatly useful for interacting with libraries expecting a certain binary
layout, and also to have a programmatic interface to code against.

[cznic]

Comment 1:

Would you mind to somehow describe what MakeStruct is/does?

[sbinet]

Comment 2:

sure (actually, for consistency with other functions in 'reflect', let's call it
StructOf):
func StructOf(name string, fields []StructField) Type
this would generate a reflect.structType with the correct field names, offsets and
alignment informations.
each field in []StructField would not need to be completely initialized ie: only
StructField.Name and StructField.Type would be mandatory
or perhaps introduce a new struct Field with only those mandatory informations and use
that instead of StructField ?
-s

[cznic]

Comment 3:

Thanks. IIUC, one would not be able to directly reference any field of the run-time
defined struct, like in 'InstanceName.FieldName', because the type is not known at
compile time. The fields would be gettable/settable only through reflect again.
I don't think Go needs such functionality. If eg., as you wrote, precise binary layout
is required by a non Go library, I think some C glue should be written instead. That's
fast and accessible in both of the worlds. Abusing reflection for run-time type-unsafe
bypassing of tasks which can be type-safely handled at compile time seems like a bad
idea to me.

[sbinet]

Comment 4:

well, another use-case would be a reflect-based go interpreter.
but I am not sure that would convince you either :)
-s

[ianlancetaylor]

Comment 5:

Labels changed: added priority-later, removed priority-triage.

Owner changed to @rsc.

[robpike]

Comment 6:

Status changed to Accepted.

[rsc]

Comment 7:

Labels changed: added go1.3maybe.

[rsc]

Comment 8:

Labels changed: added release-none, removed go1.3maybe.

[rsc]

Comment 9:

Labels changed: added repo-main.

[glycerine]

Comment 10:

I'd like (it would be terrific) to be able to create structs from data loaded at runtime.

[gopherbot]

Comment 11 by bprosnitz@google.com:

It would be very useful for my current project to be able to create structs type objects
from a type description at runtime.

[gopherbot]

Comment 12 by carlchatfield:

Here is a patch for this. I'm new to compiler foo so It would be good if someone could
if someone could check this over. Especially the field alignment and GC.

Attachments:

1. structof.patch (10651 bytes)

[glycerine]

Comment 13:

woot! excellent!! I'll try it out shortly.

[ianlancetaylor]

Comment 14:

Thanks for working on this.  Please submit patches through the process described at
http://golang.org/doc/contribute.html.  Thanks.
I think that the comparison algorithm isn't quite right.  I don't think it will work for
struct { a, b string }.

[adonovan]

Comment 15:

I agree this would be a useful feature for programs that create a graph of compact
objects based on a dynamically loaded schema, e.g. a .proto file or XML schema, or the
stack frames of an interpreter.
0xjnml: it needn't be type-unsafe.  So long as MakeStruct rejects structs whose fields
overlap, you can't create pointer/uintptr union types.
MakeStruct should probably also reject anonymous fields, since the resulting struct
could have a non-empty method-set but would have compiler-generated wrapper methods for
them.

[glycerine]

Comment 16:

I see Carl did some additional work here:
https://code.google.com/r/carlchatfield-go-arrayof-structof/source/list
Could I ask: Carl, where does this stand? Is it done?
I tried the above patch from Mar 2, made it work with a little adaptation. I think I'm
seeing the string-comparison issue that Ian pointed out. For example this test fails
(below). I'm not sure why it fails or how to fix it (any insight Ian--what was your
observation?)
Just wanted to ask where things stood with Carls' work and get everyone else's input on
where things stand. Is development on this going on elsewhere?
Thanks.
Jason
// failing test:
package main
import (
    "fmt"
    "reflect"
    "testing"
)
func TestStructOfTwoStrings(t *testing.T) {
=>  st := reflect.StructOf([]reflect.StructField{{Name: "a", Type: reflect.TypeOf("")},
{Name: "b", Type: reflect.TypeOf("")}})
    v1 := reflect.New(st).Elem()
    v2 := reflect.New(st).Elem()
    i1 := v1.Interface()
    i2 := v2.Interface()
    if i1 != i2 {
        fmt.Printf("constructed struct %v not equal to itself", v1.Interface())
        panic("but it should be!")
    }
    v1.FieldByName("a").Set(reflect.ValueOf("hello"))
    if i1, i2 := v1.Interface(), v2.Interface(); i1 == i2 {
        fmt.Printf("constructed structs %v and %v should not be equal", i1, i2)
        panic("but they were!")
    }
    v2.FieldByName("a").Set(reflect.ValueOf("hello"))
    if v1 != v2 {
        fmt.Printf("v1 = %#v\n", v1)
        fmt.Printf("v2 = %#v\n", v2)
        panic("v1 and v2 should be value equal now! Shouldn't they?") // currently failing here.                                                                                         
    }
    st = reflect.StructOf([]reflect.StructField{{Name: "X", Tag: "x", Type: reflect.TypeOf([]int(nil))}})
    v1 = reflect.New(st).Elem()
    shouldPanic(func() { _ = v1.Interface() == v1.Interface() })
}

[ianlancetaylor]

Comment 17:

My observation at the time was that the struct comparison code either did a straight
memory comparison or failed to compare.  But a struct with a string field requires an
element by element comparison: strings are comparable, but you can't compare them using
a straight memory comparison.  The same is true for float, complex, interface, and
structs or arrays with fields of the those types.

[glycerine]

Comment 18:

Thanks Ian. I think I see. Struct comparison needs to create a per-field tree that
holds, recursively, a comparison function per field. Where is this done for compile-time
created structs?  Can we not just re-use that code?
Thanks.
Jason

[glycerine]

Comment 19:

Nevermind my question. I heard from Carl and he's already finished the implementation in
his branch (mentioned above). He's waiting for 1.3 to land before submitting it for code
review.

[gopherbot]

Comment 20:

CL https://golang.org/cl/101330055 mentions this issue.

[sbinet]

fixed by CL https://golang.org/cl/101330055
( @adg : not sure if that's a bug in gopherbot. shouldn't have this been automatically closed?)

[minux]

That CL doesn't even pass code review yet, so this issue is not fixed.

[sbinet]

oh. right. just ignore me :}

[randall77]

ArrayOf just landed, https://go-review.googlesource.com/#/c/4111/

Getting equals and hash correct is one of the hard parts of StructOf. The ArrayOf change makes the equal and hash functions from closures. The same technique could be used here.

(Closures for equal/hash became available as of https://go-review.googlesource.com/#/c/2392/)

[sbinet]

I'll give it a try tomorrow.

[gopherbot]

CL https://golang.org/cl/9251 mentions this issue.

[gopherbot]

CL https://golang.org/cl/23170 mentions this issue.

[ben-clayton]

Is this now done?

[bradfitz]

Seems like it.

[mdempsky]

@ben-clayton Not completely. It's still not possible to embed types with non-exported methods.

[bradfitz]

Or apparently recursive structs: #20013

[bradfitz]

@mdempsky, I think embedding types with non-exported methods should probably be a new bug rather than reusing this one for four years.

[mdempsky]

@bradfitz Okay by me. Tracking in #20015.