ahatanak added a comment.

In D62988#1538577 <https://reviews.llvm.org/D62988#1538577>, @rsmith wrote:

> How do you write correct (non-leaking, non-double-freeing, 
> non-releasing-invalid-pointers) code with this attribute? For example, 
> suppose I have a `__strong` union member: does storing to it release the old 
> value (which might be a different union member)? If so, how do you work 
> around that? 
> https://clang.llvm.org/docs/AutomaticReferenceCounting.html#ownership-qualified-fields-of-structs-and-unions
>  should be updated to say what happens. If manual reference counting code is 
> required to make any use of this feature correct (which seems superficially 
> likely), is this a better programming model than `__unsafe_unretained`?


I should first make it clear that this attribute is a dangerous feature that 
makes it easier for users to write incorrect code if they aren't careful.

To write correct code, users have to manually (I don't mean 
manual-retain-release, this is compiled with ARC) do assignments to the fields 
that are annotated with the attribute to patch up the code generated by the 
compiler since the compiler isn't generating the kind of special functions it 
generates for non-trivial structs. For example:

  union U1 {
    id __weak __attribute__((non_trivial_union_member)) a;
    id __attribute__((non_trivial_union_member)) b;
  };
    
  id getObj(int);
  
  void foo() {
    union U1 u1 = { .b = 0}; // zero-initialize 'b'.
    u1.b = getObj(1); // assign to __strong field 'b'.
    u1.b = getObj(2); // retain and assign another object to 'b' and release 
the previously referenced object.
    u1.b = 0; // release the object.
    id t = getObj(3);
    u1.a = t; // assign to __weak field 'a'.
    u1.a = 0; // unregister as a __weak object.
  }
  
  struct S1 {
    union U1 f0;
    int f1;
  };
  
  void foo1() {
    struct S1 s1 = { .f0 = 0};
    s1.f0.a = getObj(4);
    struct S1 s2 = s1; // this is just a memcpy
    s2.f0.a = s1.f0.a;  // manually copy __weak field 'a'.
    s2.f0.a = 0;  // unregister as a __weak object.
  }

Storing to a `__strong` union member does release the old value. The old value 
can be a different union member and I think that should be fine as long as the 
other member is also `__strong`. If the other member has a different lifetime 
qualifier, I believe it has to be nulled out before assigning the new object as 
shown in the example above.

> Unions with non-trivial members are only permitted in C++11 onwards; should 
> we allow the attribute in C++98 mode? But more than that, unions with 
> non-trivial members require user-provided special members in C++11 onwards, 
> meaning that a union using this attribute in C would have a different calling 
> convention than the "natural" equivalent in C++. So I'm also wondering if we 
> should allow this in all language modes.

I think we can allow this attribute in C++ mode including C++98. One thing to 
note is that the compiler creates temporaries users can't directly access when 
passing/returning unions with non-trivial members to/from functions or 
capturing them as a block capture. In those cases, users won't be able to patch 
up the code generated by the compiler, so it would be incorrect to pass a union 
whose active member is the one annotated with `non_trivial_union_member`.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62988/new/

https://reviews.llvm.org/D62988



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to