Proposal is to add a @cpp_mapping as IDL4toC++ has, to enable users to select a mapping of IDL struct to a C++ struct (that mapping itself has to be added here also)
In user code I want to use constexpr for IDL fixed values, for example
constexr F pi
{3.142857};
But in order for that to work a constexpr constructor has to be available, the spec should define that
CORBA::CustomerMarshal, should be CORBA::CustomMarshal,
CORBA::CustomerMarshal should be CORBA::CustomMarshal
In order to get a type system which is CORBA independent we propose to change the exception which can be thrown by the union from CORBA::BAD_PARAM to std::invalid_argument. The impact for users is minimal as they can except std exceptions from the other std types they already use through this language mapping.
The specification lacks a description how to map the @external annotation
The IDL to C++11 spec lacks a mapping for the min/max/range annotations
The spec says "In case the union has an implicit default member it is initialized to that case", but it doesn't specify what is an implicit default member, maybe add "i.e., if the union does not have a default case and not all permissive discriminator values are listed"
The default annotation is not just for struct members, also for exception/union members but it can also be applied to a type, so also for example attributes of an interface/component/connector can have a default, all code generation related to that should also use the default whenever one is needed (for example starter code generation)
All IDL/C++ that is used in the text should be have a different font like done in the other parts of the spec, that is for example int8_t/uint8_t/Base::Base/
There shouldn't be a semi colon after the namespace closure, this is done in more places in the spec, search for "namespace", closing should be "}" and not "};", see chapters 6.3/6.5/6.23.1/6.26.2/6.27.2/6.28/6.29
For the following places the declared destructors should be using override and not virtual as they override the base class destructor. This should happen to:
This chapter says:
The Fixed(std::string&) constructor converts a string representation of a fixed-point literal, with an optional leading sign (+ or -) and an optional trailing ‘d’ or ‘D’, into a real fixed-point value.
But there is no constructor that accepts a non-const std::string&, the text should say
The Fixed(const std::string&) constructor converts a string representation of a
fixed-point literal, with an optional leading sign (+ or -) and an optional trailing ‘d’ or ‘D’, into a real fixed-point value.
Correct the spec text to match the source code
Add the missing "const" as described in the issue.
Double "to to" in the first sentence:
IDL bitset types are mapped to to C++ structs that meet the C++11 requirements for aggregate initialization.
Should be
IDL bitset types are mapped to C++ structs that meet the C++11 requirements for aggregate initialization.
Fix typo
As per issue description
The spec should mention how the IDL4-mentioned annotations impact the mapping, I think the spec should list at least @value, @default_literal, @default, @range @min @max @bit_bound @external
See IDL4 to CPP and the IDL4 spec itself
Add mappings for some more IDL 4 Standardized Annotations
New mappings for @value, @default_literal, @default, and @bit_bound – in each case drawing from uses in the beta IDL4-to-C++ mapping and/or XTypes.
In the specification an addition was made about the default argument when the union member has multiple legal discriminator values:
Union members associated with the "default" label and those associated with multiple labels may have multiple legal discriminator values. For these members, the modifiers (those that return void) have a second parameter which has the discriminator's type. This parameter has a default argument. For members associated with multiple labels, the value of the default argument is the value of the case label that appears first in IDL. For members associated with the default label, the value of the default argument is an implementation-defined value that selects the member.
There is nothing in the text that describes what happens when a discriminator value is passed which doesn't belong to the member, this is implicitly in the example code further down "u.obj(a, 2)", but this should be explicit in the text also, passing a not allowed discriminator should result in a BAD_PARAM exception
Add description of error handling for 2-parameter union modifier functions
To be consistent with the existing parts of the spec, make it clear that invalid parameters result in a BAD_PARAM exception.
Instead of `VariableExt(const Variable&, bool);` it should be `VariableExt(Variable, bool);` so that in the implementation we can use std::move, this is as all complex types are passed into the constructor of a struct
In example code, function signature of constructors is non-normative
In section 6.31.3 make it clear that the specific types of the constructor parameters (if they use pass by value or pass by const-reference) is up to the implementation.
The bitmask mapping is very basic, it doesn't give any type safety or more modern API. The spec should be improved, the <X>Bits type should be removed, only one type should be used. Also when there are multiple bitmasks with the same bitfield name they could result in a name clash. Maybe map to a enum class and add the operators ~,|,^,& for the enum class as inline operations to modify the enum class as a bitmask. At that moment the Bit type can be removed.
When time permits I will make a proof of concept for TAOX11
Defer breaking change to the bitmask mapping
The existing mapping in IDL-to-C++11 is based on DDS XTypes where it has been in a formal standard for a number of years.
There are certainly reasons to have a mapping that makes better use of the C++ type system, but this would come at a cost in terms of divergence from OMG precedent.
For some use cases it would be more efficient to map a IDL map to a std::unordered_map (see https://en.cppreference.com/w/cpp/container/unordered_map), maybe standardize an annotation which can be used to override the mapping to std::map to the type as specified by the annotation?
Defer
Other languages have similar issues like using TreeMap or HashMap in Java.
Defer resolution on this until standard annotation can be provided in the IDL 4 spec or the IDL Working Group has a common approach to use across language mappings.
using M2 = IDL::bounded_map<std::string, T, 20;
should be
using M2 = IDL::bounded_map<std::string, T, 20>;
Typo is not in base document
ptc/21-11-02 doesn't have this typo
The section says:
"For all primitive types, enums, and
reference types, an in argument A of type P, that argument is passed as P. For all other types, an in argument A of type P is
passed as const P&. For an inout and out argument it is passed as P&. If we return a type of P, it is returned as P."
This is the only use of "primitive types" in the spec. "Basic Data Types" should be preferred. Would suggest:
"For all Basic Data Types [link/ref to 6.6 Mapping for Basic Data Types], enums [link/ref to 6.9 Mapping for Enums], and
reference types [link/ref to 6.7.1 Reference Types], an in argument A of type P, that argument is passed as P. For all other types, an in argument A of type P is
passed as const P&. For an inout and out argument it is passed as P&. If we return a type of P, it is returned as P."
Update text in Arg Passing to refer to Basic Data Types
Update as specified in parent issue
The spec defines the type trait bit_bound as `std::integral_constant<uint32_t, b>`, but as bit_bound is maximal of 64, this could be `std::integral_constant<uint8_t, b>`
Update type used for bit_bound in bit mask traits
The integral_constant template from the C++ standard library should be instantiated with uint8_t instead of uint32_t
The bit_bound annotation can be applied to the enum, so the bit_bound and underlying_type traits should also be added to the enum
Add type traits for enums
Including this in the resolution to CPP1117-13.
For the optional mapping example a `void age(IDL::optional<float>&&)` is lacking, like described for some types in chapter 6.14
For the example code, for the setter, the std::optional should be passed as const&, see https://stackoverflow.com/questions/69277649/does-passing-an-stdoptionalt-by-reference-actually-save-copying
Within the text of chapter 6.31/6.32 (IDL4) all C++/IDL should use a different font to make it clear it is code, just as in the other chapters.
For anyone using the class mapping of a structure the spec should also provide a class mapping for a bitset, that way they have a consistent API. Also why is a inherited bitset not mapped to C++ inheritance, by removing the inheritance the inheritance relation can't be used in C++ also
C++20 modules look very interesting for sharing declarations/definitions based on IDL (see https://en.cppreference.com/w/cpp/language/modules), would be good when this spec has support for C++20 modules