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

Thrift compiler should be able to output c++ Aggregate types

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Open
    • Major
    • Resolution: Unresolved
    • 0.16.0, 0.17.0
    • None
    • C++ - Compiler
    • None

    Description

      If we have a thrift struct defined like this:

      struct thingamabob {
          1: i64 a = 0
          2: i64 b = 0
          3: i64 c = 0
      }

      Then the generated code contains a few constructors and assignment operators that look like this:
      thingamabob(const thingamabob&) noexcept;
      thingamabob&operator=(const thingamabob&) noexcept;
      thingamabob() noexcept: a(0),b(0),c(0) {}
      The issue here is that while this class could be an AggregateType and support aggregate initialization, the way these defaults are generated prevent it.

      If instead, thrift used default initializers like this where the the member objects are defined:
      int64_t a{0};
      int64_t b{0};
      int64_t c{0};
      We could then initialize thrift structs with initializer lists, expect 
      static_assert(std::is_aggregate_v<type>) to work, and we'd still have default construction and copying.
       
      For this to work, the "templates" option passed to the c++ compiler should remove the virtual keyword from the generated printTo functions, and the generated __isset structs would also need to be changed similarly to look like:
      typedef struct_thingamabob__isset {
      bool a :1{false};
      bool b :1{false};
      bool c :1{false};
      } thingamabob_isset;
       
      Happy to take a stab at this and wanted to check for feedback first.

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              bgemmill Benjamin Gemmill
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:

                Time Tracking

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