Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-4868

Go bug for optional sets with a default value

    XMLWordPrintableJSON

    Details

    • Type: Task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.12.0
    • Fix Version/s: None
    • Component/s: Go - Compiler
    • Labels:
      None

      Description

      There's a bug in the Thrift Go generator that happens when optional set types are used with a default field. For example, a simple struct like this:

      struct SetPointerTest {
        1: optional set<string> with_default = [ "test" ]
      }
      

      Generates a struct like this

      type SetPointerTest struct {
        WithDefault *[]string `thrift:"with_default,1" db:"with_default" json:"with_default"`
      }
      

      And associated Go code with a line like this:

      if reflect.DeepEqual(*p.WithDefault[i],*p.WithDefault[j]) {
      

      Which fails with an error

      gen-go/setpointertest/SetPointerTest.go:125:44: invalid operation: p.WithDefault[i] (type *[]string does not support indexing)
      

      Thrift optional sets only get translated to Go pointer values (causing this error) when a default value is set. This is because the t_go_generator::is_pointer_field method has some logic that returns true if the set has a default value. I'm unsure why this is the case; it seems simpler to not change the type when there's a default value. A Golang slice is a pointer value by default, so nil can still be used for optional fields. I'd be super interested to understand why the code was written this way.

      However, since changing the pointer behavior would be a version-incompatible change, I propose we do the simple thing for now to fix Go compilation. I'll submit a patch with a test shortly.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                johnboiles John Boiles
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 20m
                  20m