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

Incomplete records generation for Erlang

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • None
    • 0.9
    • Erlang - Compiler
    • Gentoo GNU/Linux, Erlang OTP R15B

    • Patch Available

    Description

      Thrift compiler for erlang generates incomplete records. It uses erlang string() type for thrift type binary(), and also has not sense to optional values when generates record fields types. Erlang Dialyzer prints warnings on these records so it should be fixed.

      Attachments

        Issue Links

          Activity

            mtreskin Max Treskin added a comment -

            Patch attached

            mtreskin Max Treskin added a comment - Patch attached
            mtreskin Max Treskin added a comment -

            Hey, anybody here?

            mtreskin Max Treskin added a comment - Hey, anybody here?

            I'm not clear how things are broken, do you have an example .thrift file and an example of how running it with the dialyzer fails?

            I tried

            struct StructA
            {
              1: string a;
              2: binary b;
              3: optional string c;
              4: optional binary d;
              5: required string e;
              6: required binary f;
              7: string g = "foo";
            }
            

            Which generates

            -record(structA, {a = undefined :: string(),
                              b = undefined :: string(),
                              c = undefined :: string(),
                              d = undefined :: string(),
                              e = undefined :: string(),
                              f = undefined :: string(),
                              g = "foo" :: string()}).
            

            Which arguably is not quite correct, but doesn't cause the dialyzer to fail with an error, so I'm not sure what the issue your patch fixes is. Can you include some information on how to reproduce and test this?

            Also, I'm wondering if the type signatures might cause issues in a server. Right not when a server gets a record which fields which are "strings" the fields actually come in as binaries (assumably as an optimization as you might not need them as "strings", so dialyzer might complain about that, not sure).

            djnym Anthony Molinaro added a comment - I'm not clear how things are broken, do you have an example .thrift file and an example of how running it with the dialyzer fails? I tried struct StructA { 1: string a; 2: binary b; 3: optional string c; 4: optional binary d; 5: required string e; 6: required binary f; 7: string g = "foo" ; } Which generates -record(structA, {a = undefined :: string(), b = undefined :: string(), c = undefined :: string(), d = undefined :: string(), e = undefined :: string(), f = undefined :: string(), g = "foo" :: string()}). Which arguably is not quite correct, but doesn't cause the dialyzer to fail with an error, so I'm not sure what the issue your patch fixes is. Can you include some information on how to reproduce and test this? Also, I'm wondering if the type signatures might cause issues in a server. Right not when a server gets a record which fields which are "strings" the fields actually come in as binaries (assumably as an optimization as you might not need them as "strings", so dialyzer might complain about that, not sure).

            I believe the fix for THRIFT-1532 also fixes this. I went a slightly different direction, so see THRIFT-1532 for details. Also please try out the patch and let me know if doesn't work in some cases.

            djnym Anthony Molinaro added a comment - I believe the fix for THRIFT-1532 also fixes this. I went a slightly different direction, so see THRIFT-1532 for details. Also please try out the patch and let me know if doesn't work in some cases.
            hudson Hudson added a comment -

            Integrated in Thrift #426 (See https://builds.apache.org/job/Thrift/426/)
            THRIFT-1532/THRIFT-1475 - fix record generation for erlang (Revision 1303663)

            Result = SUCCESS
            molinaro : http://svn.apache.org/viewvc/?view=rev&rev=1303663
            Files :

            • /thrift/trunk/compiler/cpp/src/generate/t_erl_generator.cc
            • /thrift/trunk/lib/erl/Makefile.am
            • /thrift/trunk/lib/erl/test/Thrift1475.thrift
            hudson Hudson added a comment - Integrated in Thrift #426 (See https://builds.apache.org/job/Thrift/426/ ) THRIFT-1532 / THRIFT-1475 - fix record generation for erlang (Revision 1303663) Result = SUCCESS molinaro : http://svn.apache.org/viewvc/?view=rev&rev=1303663 Files : /thrift/trunk/compiler/cpp/src/generate/t_erl_generator.cc /thrift/trunk/lib/erl/Makefile.am /thrift/trunk/lib/erl/test/Thrift1475.thrift

            People

              djnym Anthony Molinaro
              mtreskin Max Treskin
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - 1h
                  1h
                  Remaining:
                  Remaining Estimate - 1h
                  1h
                  Logged:
                  Time Spent - Not Specified
                  Not Specified