dedis / protobuf

Reflection-based Protocol Buffers for Go
GNU General Public License v2.0
76 stars 15 forks source link

Failing test #30

Closed lbarman closed 6 years ago

ineiti commented 7 years ago

yes, that's a known limitation. As far as I understand, protobuf doesn't define two-dimensional arrays or maps of arrays. What you need to do is to make an array of structures with an array:

type DataStruct struct {
        DType []string
}
type Msg struct {
    Data []*DType
}

We can:

  1. return an error if the protobuf encounters an unsupported field-type
  2. write our own protobuf-extension
  3. find how protobuf does this

I think 1. should be the most appropriate.

lbarman commented 7 years ago

Yes, I understand why now. 1 is reasonnable and easy to fix on my side.

ineiti commented 6 years ago

I think there is a test in Encode for the case of 2-dimensional arrays. Else we should return an error if we encounter a slice of slice.

aounleonardo commented 6 years ago

In encode.go in sliceReflect(key, slval) we are already doing this check:

    default: // Write each element as a separate key,value pair
        t := slval.Type().Elem()
        if t.Kind() == reflect.Slice || t.Kind() == reflect.Array {
            subSlice := t.Elem()
            if subSlice.Kind() != reflect.Uint8 {
                panic("protobuf: no support for 2-dimensional array except for [][]byte")
            }
        }

        for i := 0; i < sllen; i++ {
            en.value(key, slval.Index(i), TagNone)
        }
        return
    }

and we are passing through it. However, we cannot return an error here since the method returns void. And neither the method that calls it slice nor value, nor message so we have to go all the way back to Encode.

Should we keep the panic or add return values to the private methods?

ineiti commented 6 years ago

Sorry, didn't see this conversation. Opened an issue for that: https://github.com/dedis/protobuf/issues/46