Add metadata to TypedArray and validate at runtime

[raulsntos]

• Adds metadata to ContainerTypeValidate to validate at runtime that elements added to TypedArray fit the the element type.
• Adds metadata to TypedArray's GetTypeInfo<> so it's included in extension_api.json and language bindings can use the right element type for the Array.

As an example, the method CodeEdit::get_folded_lines that returns TypedArray<int>:

https://github.com/godotengine/godot/blob/c184105cf55614078b509b0a57c4bbd0beb62be7/scene/gui/code_edit.cpp#L1722

{
    "name": "get_folded_lines",
    "is_const": true,
    "is_vararg": false,
    "is_static": false,
    "is_virtual": false,
    "hash": 3995934104,
    "return_value": {
        "type": "typedarray::int",
+       "meta": "int32"
    }
},

[dsnopek]

Code-wise, I think this is the right approach to do these checks.

However, I think we should have a core discussion to decide if we want to have these checks. Do we want TypedArray to be specific to integer and float sizes? Or, do we only want to deal with Variant types (and so it's always int64_t and double)?

I could personally see arguments for both sides.

Getting specific with sizes potentially allows packing the data tighter when copying to/from native containers (although, inside the TypedArray, it'll always be Variants - to get packed data on both the Godot and native side the Packed*Array types are better). However, the added checks have an effect on performance on every set or get operation, for something that we might not really care about?

[raulsntos]

I think we should have a core discussion to decide if we want to have these checks.

That's fair. I said this on RC but I'll say it again here for the record, the motivation behind this PR is to generate GDExtension-based C# bindings with the correct element type for typed arrays.

The godot-cpp bindings generator is using the element type as-is. So instead of assuming int means 64-bit like everywhere else where int appears, the generated APIs use int (not int32_t or int64_t).

I also checked what godot-rust does, and they do treat int like it means 64-bit so their generated APIs use typed arrays of i64.

And this is the list of APIs where a TypedArray with an integer element type appears (grouped by element type):

• APIs with TypedArray<int>:
  • CodeEdit::get_folded_lines
  • CodeEdit::set_line_length_guidelines
  • CodeEdit::get_line_length_guidelines
  • Input::get_connected_joypads
• APIs with TypedArray<uint64_t>:
  • PhysicsTestMotionParameters2D::get_exclude_objects
  • PhysicsTestMotionParameters2D::set_exclude_objects
  • PhysicsTestMotionParameters3D::get_exclude_objects
  • PhysicsTestMotionParameters3D::set_exclude_objects
• APIs with TypedArray<int64_t>:
  • RDPipelineMultisampleState::set_sample_masks
  • RDPipelineMultisampleState::get_sample_masks

[Bromeon]

However, the added checks have an effect on performance on every set or get operation, for something that we might not really care about?

If static type information about the array is available (Array[T]), it should only be necessary to add the checks for the following element types (when converting from Variant):

• int8_t
• int16_t
• int32_t
• uint8_t
• uint16_t
• uint32_t

If the array is untyped (Array), you'd still need to query the Variant type, but this is already done today. You would only need to do an additional check (type of inserted value vs. element meta) for the above types.

There are some special cases to consider:

• int64_t -- this is the native integer type. Since all its values are perfectly representable, Array<int64_t> wouldn't need a runtime check.
• uint64_t -- it is arguable whether this type should be supported. Not all its values can be stored as int64_t, so if you TypedArray<uint64_t> to an untyped array, you'll end up with different values as elements, which can introduce subtle bugs.
• float and double. Since the native Variant type is double, it should be able to hold all values of both types. If the user decides to use float for inserting/extracting elements, should that really be an error?

---

TLDR: unless I'm missing something, using TypedArray<T> with T = int64_t, float, double should not incur performance penalty.

[dsnopek]

• float and double. Since the native Variant type is double, it should be able to hold all values of both types. If the user decides to use float for inserting/extracting elements, should that really be an error?

I'm not sure about this. double can hold more values than float in a similar way to the integer types. If we're going to say that bindings can limit TypedArray<float> to actual 32-bit floats, then I feel like we should check (or at least cast?) those to have the same limitation on the Godot side to prevent inconsistencies.

TLDR: unless I'm missing something, using TypedArray<T> with T = int64_t, float, double should not incur performance penalty.

That makes sense. If we could have an early out for int64_t and double (using likely() or unlikely()), then the performance impact would be much reduced. In the current PR, it does an extra function call for all ints/floats, and goes through a whole bunch of ifs for ints before doing nothing for int64_t.

That said, I still think we should connect with other Godot core folks outside of GDExtension, and get more input on if these "bounds checks" are even desirable. I feel like TypedArray<T> with int/float types beyond what's native to Variant could be as much an accident as a purposeful decision.