Skip to content

codegen: do not create struct temporaries unless used directly as a parameter

Val Och requested to merge v19930312/vala:less-struct-temporaries into main

Fixes #471, fixes #1501 (closed). New test added; existing tests had to be updated because of reduced use of temporaries, pass.

While this change tries to avoid breaking existing semantics for struct function arguments, I have to suggest a breaking but IMO sensible change of not trying to enforce in-parameter struct immutability. My reasoning:

  1. Consistency with classes and arrays.
    Vala's "container" types, i.e. those storing data of other types inside them, consist of: structs, simple structs (including integers), classes, and arrays. Out of these, only changes to structs and simple structs won't be propagated back to the caller if changed in a function - while this is fully expected for simple structs (they are passed by value, after all), it it less obvious for regular structs.
  2. The copy is shallow-ish.
    Consider the following code, which compiles with no warnings but crashes when ran:
class MyClass: Object {
}

struct MyStruct {
    MyClass element;
}

void replace_element (MyStruct container) {
    // original element is freed and replaced, but this is never propagated to the original struct
    container.element = new MyClass ();
}

void main () {
    var my_struct = MyStruct ();
    my_struct.element = new MyClass ();
    replace_element (my_struct);
    // segfault (and memory leak): tried to free the original element
}

Example of this being a real issue: #1452 (first point). The only way to work this around as of now is to make sure you always pass structures with pointers elsewhere in them (even if those pointers are not user-visible), which is just waiting for an accident to happen.

  1. Consistency with nullable struct parameters: contrary to what one would expect, passing in a nullable struct via a nullable parameter and changing it there will not operate on a copy, and only in that case! Demonstration based on code from point 2 (edit for clarity: the second part is parameter declaration):
  • var my_struct, MyStruct container -> copy
  • var my_struct, MyStruct? container -> copy
  • var? my_struct, MyStruct container -> copy
  • var? my_struct, MyStruct? container -> original!
Edited by Val Och

Merge request reports