The Generated _d() Setter Function Does not Allow Setting the Discriminator From the Default Value

[Progeny42]

I'm using tag v0.10.3, and generated the *.idl files using the CXX library with the idlc application.

For example, a type position_coordinate_type is a std::variant, discrimnated based on an enumeration of CARTESIAN, POLAR, and WGS84.

My understanding of the API is to call the _d() function, passing the relevant enumeration, and then use <discriminant_function>() to set the value.

For example:

position_coordinate_type positionCoordinate;

positionCoordinate._d(POLAR);
positionCoordinate.polar_position(<value>);

However, this fails because the setter for the discriminator (_d()) performs the following check:

if (!_is_compatible_discriminator(m__d, d)) {
  throw dds::core::InvalidArgumentError(
    "Discriminator value does not match current discriminator");
}

m__d is the _default_discriminator, which in this example, is CARTESIAN. Therefore, calling this function with anything but CARTESIAN will fail, meaning you are unable to change it using this function.

I don't believe the check is actually necessary in this function, because the point of the function is to change the discriminant.

Thankfully, directly setting the value via the relevant function (in this example polar_position()) also sets the discriminator.

[eboasson]

Hi @Progeny42,

Thankfully, directly setting the value via the relevant function (in this example polar_position()) also sets the discriminator.

This is actually the intended way to use unions in this C++ binding. The setter for the discriminator only exists to set the desired discriminant value in a case where multiple values map to the same value. For example, if you have:

enum E { A, B, C, D };
union U switch (E) {
  case A: case B:
    long x;
  case C: case D:
    double y;
};

and you want the discriminant value D with y=3.14, then the intended way is to do

 u.y(3.14); _d(E::D)

It is quite annoying, I agree. I'm glad I didn't define the interface 😂

[Progeny42]

Ah I see. Thanks for the quick response. I'll go refactor my implementation to suit then.