Flexible array members in ObjC classes

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
Hello all,

Currently clang hits an assertion

Assertion failed: (!fieldType->isArrayType() && "ivar of non-constant array type?"), function visitField, file llvm-project/clang/lib/CodeGen/CGObjCMac.cpp, line 5092.

when one of ObjC class instance variables is a struct with a flexible array member and ARC is enabled. This happens only with ARC because in this case we are building ivar layout and for MRC we skip it. And ivar layout supports only constant arrays.

I am considering the following options to fix this issue:

1. Prohibit ivars with flexible array members both for ARC and MRC. Pointers to such structs are still acceptable and preferred.
2. Prohibit ivars with flexible array members only for ARC.
3. Add support for ivars with flexible array members. We already accept ivars with 0-size array members and at cursory glance it looks similar. This option has 2 sub options
  3.1 Allow ivars with flexible array members anywhere.
  3.2 Allow ivars with flexible array members only as the last ivar.

My preferred option is option 1 because to my understanding structs with flexible array members are supposed to be allocated dynamically, reserving memory for the flexible array. And having them as ObjC class ivar will cause flexible arrays being 0-sized all the time. The biggest downside of this approach is that it can cause rejecting some code that is accepted today.

The most user-friendly option seems option 3 with warnings. It is permissive but still warns about the risk of overwriting other ivars. The downside is that it is extra work for code that nobody might be writing, such code is still error-prone despite the warnings, maintenance cost is potentially higher.

Incomplete code illustrating option 1 can be found at https://reviews.llvm.org/D38114 What is your opinion? What would you recommend?

Thanks,
Volodymyr Sapsai
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
> On Sep 20, 2017, at 6:16 PM, Volodymyr Sapsai <[hidden email]> wrote:
> Hello all,
>
> Currently clang hits an assertion
>
> Assertion failed: (!fieldType->isArrayType() && "ivar of non-constant array type?"), function visitField, file llvm-project/clang/lib/CodeGen/CGObjCMac.cpp, line 5092.
>
> when one of ObjC class instance variables is a struct with a flexible array member and ARC is enabled. This happens only with ARC because in this case we are building ivar layout and for MRC we skip it. And ivar layout supports only constant arrays.
>
> I am considering the following options to fix this issue:
>
> 1. Prohibit ivars with flexible array members both for ARC and MRC. Pointers to such structs are still acceptable and preferred.
> 2. Prohibit ivars with flexible array members only for ARC.
> 3. Add support for ivars with flexible array members. We already accept ivars with 0-size array members and at cursory glance it looks similar. This option has 2 sub options
>  3.1 Allow ivars with flexible array members anywhere.
>  3.2 Allow ivars with flexible array members only as the last ivar.
>
> My preferred option is option 1 because to my understanding structs with flexible array members are supposed to be allocated dynamically, reserving memory for the flexible array. And having them as ObjC class ivar will cause flexible arrays being 0-sized all the time.

Objective-C objects are allocated dynamically, and it's absolutely possible to allocate extra storage in an object and then access that via a trailing flexible or zero-length array.  It is, of course, somewhat perilous, but that's true of flexible/zero-length array members of structs, too.

> The biggest downside of this approach is that it can cause rejecting some code that is accepted today.
>
> The most user-friendly option seems option 3 with warnings. It is permissive but still warns about the risk of overwriting other ivars. The downside is that it is extra work for code that nobody might be writing, such code is still error-prone despite the warnings, maintenance cost is potentially higher.
>
> Incomplete code illustrating option 1 can be found at https://reviews.llvm.org/D38114 What is your opinion? What would you recommend?

I think option 3 is the right way to go: at a minimum, fix the bug in the layout-string computation by just ignoring flexible array members.  Adding additional semantic checks to ensure that the flexible array is the last ivar (whether explicit or synthesized) seems like a good idea, too, but it's not mandatory.

John.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
On 21 Sep 2017, at 02:51, John McCall via cfe-dev <[hidden email]> wrote:
>
> Objective-C objects are allocated dynamically, and it's absolutely possible to allocate extra storage in an object and then access that via a trailing flexible or zero-length array.  It is, of course, somewhat perilous, but that's true of flexible/zero-length array members of structs, too.

This is a lot more perilous with the non-fragile ABI.  In classical Objective-C, classes were lowered to structs and so all of these tricks were used (indeed, allocating objects with extra space was explicitly supported by all of the common allocation mechanisms).  Adding a flexible array member at the end was not a problem, because anyone subclassing your class could see the structure layout in the header and know that subclassing was probably dangerous.

The only place where it should be allowed at all is in the last ivar in a class, and the only place it should be allowed in the non-fragile ABI is the last ivar of a class that cannot be subclassed (which, given that objc_subclassing_restricted doesn’t seem to be documented and is not supported by older compilers, is not something that’s easy to enforce).

David

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
My initial reaction was, a flexible array as an ivar is dumb. Flexible
array ivars must appear at the end of the struct (AFAIK). Then you can't
subclass such a class anymore.

My second reaction, was to think that there are class cluster classes
with allocate a flexible amount of memory (say string classes) that
aren't ususally subclassed. Who am I to say, that using a convenient
flexible array there can't be done (anymore) ?

I would think a warning would be nice for classes who try to subclass a
class with a flexible array ivar if they add another property or ivar.

Ciao
    Nat!



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
On 21 Sep 2017, at 10:07, Nat! via cfe-dev <[hidden email]> wrote:
>
> My second reaction, was to think that there are class cluster classes with allocate a flexible amount of memory (say string classes) that aren't ususally subclassed. Who am I to say, that using a convenient flexible array there can't be done (anymore) ?

This is more or less how a number of the GNUstep Foundation classes work, using the extra data allocated with class_createInstance() and doing some pointer arithmetic.  Flexible array members would possibly simplify the code, but they’re not needed for this to work.

> I would think a warning would be nice for classes who try to subclass a class with a flexible array ivar if they add another property or ivar.

The problem is that you can’t do this with the non-fragile ABI.  You can write the flexible array member in the @implementation of a class and the person writing the subclass won’t see it.

To make this safe, the minimum set of constraints that you’d need would be:

 - The ivar is the last one
 - The ivar is declared in the @interface
 - The class does not declare any ivars in the @implementation
 - Subclassing a class whose @interface ends with a flexible member is an error

With this set of constraints, flexible array members would be no less safe than using them in C.

David

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
David Chisnall schrieb:

> On 21 Sep 2017, at 10:07, Nat! via cfe-dev<[hidden email]>  wrote:
>> My second reaction, was to think that there are class cluster classes with allocate a flexible amount of memory (say string classes) that aren't ususally subclassed. Who am I to say, that using a convenient flexible array there can't be done (anymore) ?
>
> This is more or less how a number of the GNUstep Foundation classes work, using the extra data allocated with class_createInstance() and doing some pointer arithmetic.  Flexible array members would possibly simplify the code, but they’re not needed for this to work.
>
>> I would think a warning would be nice for classes who try to subclass a class with a flexible array ivar if they add another property or ivar.
>
> The problem is that you can’t do this with the non-fragile ABI.  You can write the flexible array member in the @implementation of a class and the person writing the subclass won’t see it.
>
> To make this safe, the minimum set of constraints that you’d need would be:
>
>   - The ivar is the last one
>   - The ivar is declared in the @interface
>   - The class does not declare any ivars in the @implementation
>   - Subclassing a class whose @interface ends with a flexible member is an error
>

That'd mean you couldn't write a subclass that doesn't add an ivar. Or a
subclass that overrides the flexible allocation methods but adds some
ivars and does something completely else. That's why I think a warning
would be nice, but not an error.

Obviously I am just speaking from my fragile-ABI only perspective :)

Ciao
    Nat!

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev

> On 21 Sep 2017, at 11:04, Nat! via cfe-dev <[hidden email]> wrote:
>
> David Chisnall schrieb:
>> On 21 Sep 2017, at 10:07, Nat! via cfe-dev<[hidden email]>  wrote:
>>> My second reaction, was to think that there are class cluster classes with allocate a flexible amount of memory (say string classes) that aren't ususally subclassed. Who am I to say, that using a convenient flexible array there can't be done (anymore) ?
>>
>> This is more or less how a number of the GNUstep Foundation classes work, using the extra data allocated with class_createInstance() and doing some pointer arithmetic.  Flexible array members would possibly simplify the code, but they’re not needed for this to work.
>>
>>> I would think a warning would be nice for classes who try to subclass a class with a flexible array ivar if they add another property or ivar.
>>
>> The problem is that you can’t do this with the non-fragile ABI.  You can write the flexible array member in the @implementation of a class and the person writing the subclass won’t see it.
>>
>> To make this safe, the minimum set of constraints that you’d need would be:
>>
>>  - The ivar is the last one
>>  - The ivar is declared in the @interface
>>  - The class does not declare any ivars in the @implementation
>>  - Subclassing a class whose @interface ends with a flexible member is an error
>>
>
> That'd mean you couldn't write a subclass that doesn't add an ivar.

That’s true.  Subclasses that don’t add any state should be okay, though this then becomes a bit harder because you need a recursive check that no superclasses end with a fragile array member.

> Or a subclass that overrides the flexible allocation methods but adds some ivars and does something completely else.

This is very fragile, as it relies on knowing that none of the superclass methods will expect the flexible array member to have allocated space and will cause overlapping ivars.

> That's why I think a warning would be nice, but not an error.
>
> Obviously I am just speaking from my fragile-ABI only perspective :)

I don’t believe that fragile ABI usage should dictate the language, given that existing recent language features (e.g. ivar declarations in @implementations and class extensions) rely on the non-fragile behaviour.

David

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
David Chisnall schrieb:
>>
>> Obviously I am just speaking from my fragile-ABI only perspective :)
>
> I don’t believe that fragile ABI usage should dictate the language, given that existing recent language features (e.g. ivar declarations in @implementations and class extensions) rely on the non-fragile behaviour.
>

My main objection with the the error instead of the warning is, that by
making it an error, you are implicitly shoving 'final' into Objective-C
through the backdoor.

@interface Foo
{
    char final[];  // can't subclass anymore :P
}
@end


I am perfectly fine with the non-fragile/ARC part of Objective-C going
hogwild, with whatever they want to do. I am obviously unhappy, if
that's to the detriment of non-fragile. I don't really think that's
"dictating" though. :)

Ciao
    Nat!

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
> On Sep 21, 2017, at 1:30 AM, David Chisnall <[hidden email]> wrote:
> On 21 Sep 2017, at 02:51, John McCall via cfe-dev <[hidden email]> wrote:
>>
>> Objective-C objects are allocated dynamically, and it's absolutely possible to allocate extra storage in an object and then access that via a trailing flexible or zero-length array.  It is, of course, somewhat perilous, but that's true of flexible/zero-length array members of structs, too.
>
> This is a lot more perilous with the non-fragile ABI.  In classical Objective-C, classes were lowered to structs and so all of these tricks were used (indeed, allocating objects with extra space was explicitly supported by all of the common allocation mechanisms).  Adding a flexible array member at the end was not a problem, because anyone subclassing your class could see the structure layout in the header and know that subclassing was probably dangerous.
>
> The only place where it should be allowed at all is in the last ivar in a class, and the only place it should be allowed in the non-fragile ABI is the last ivar of a class that cannot be subclassed (which, given that objc_subclassing_restricted doesn’t seem to be documented and is not supported by older compilers, is not something that’s easy to enforce).

Yeah, I don't think we can reasonably enforce anything about subclasses, except maybe warning if we see that we're adding an ivar (explicitly or via synthesis) to a class that's known to have a flexible array member.  Of course that means that the protection will be incomplete.

John.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
On Sep 21, 2017, at 10:07, John McCall <[hidden email]> wrote:

On Sep 21, 2017, at 1:30 AM, David Chisnall <[hidden email]> wrote:
On 21 Sep 2017, at 02:51, John McCall via cfe-dev <[hidden email]> wrote:

Objective-C objects are allocated dynamically, and it's absolutely possible to allocate extra storage in an object and then access that via a trailing flexible or zero-length array.  It is, of course, somewhat perilous, but that's true of flexible/zero-length array members of structs, too.

This is a lot more perilous with the non-fragile ABI.  In classical Objective-C, classes were lowered to structs and so all of these tricks were used (indeed, allocating objects with extra space was explicitly supported by all of the common allocation mechanisms).  Adding a flexible array member at the end was not a problem, because anyone subclassing your class could see the structure layout in the header and know that subclassing was probably dangerous.

The only place where it should be allowed at all is in the last ivar in a class, and the only place it should be allowed in the non-fragile ABI is the last ivar of a class that cannot be subclassed (which, given that objc_subclassing_restricted doesn’t seem to be documented and is not supported by older compilers, is not something that’s easy to enforce).

Yeah, I don't think we can reasonably enforce anything about subclasses, except maybe warning if we see that we're adding an ivar (explicitly or via synthesis) to a class that's known to have a flexible array member.  Of course that means that the protection will be incomplete.

John.

Thanks everybody for their input. I plan to update the patch with different test cases so it's easier to discuss. But so far my summary is the following.

For flexible array members we allow them only as the last ivar in @interface, with no ivars in @implementation. If it's not last, if it is in @implementation, if there are more ivars in @implementation - that's an error.

For variable sized types (i.e. structs with last member a flexible array) we don't error but warn if we can detect more ivars after it (in @interface or in @implementation). If @implementation ivars end with variable sized ivar, no error or warning.

For subclassing we don't error but warn if we can detect more ivars after a flexible array member or after a variable sized type member.

Thanks,
Volodymyr

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
On Sep 21, 2017, at 12:04 PM, Volodymyr Sapsai <[hidden email]> wrote:
On Sep 21, 2017, at 10:07, John McCall <[hidden email]> wrote:

On Sep 21, 2017, at 1:30 AM, David Chisnall <[hidden email]> wrote:
On 21 Sep 2017, at 02:51, John McCall via cfe-dev <[hidden email]> wrote:

Objective-C objects are allocated dynamically, and it's absolutely possible to allocate extra storage in an object and then access that via a trailing flexible or zero-length array.  It is, of course, somewhat perilous, but that's true of flexible/zero-length array members of structs, too.

This is a lot more perilous with the non-fragile ABI.  In classical Objective-C, classes were lowered to structs and so all of these tricks were used (indeed, allocating objects with extra space was explicitly supported by all of the common allocation mechanisms).  Adding a flexible array member at the end was not a problem, because anyone subclassing your class could see the structure layout in the header and know that subclassing was probably dangerous.

The only place where it should be allowed at all is in the last ivar in a class, and the only place it should be allowed in the non-fragile ABI is the last ivar of a class that cannot be subclassed (which, given that objc_subclassing_restricted doesn’t seem to be documented and is not supported by older compilers, is not something that’s easy to enforce).

Yeah, I don't think we can reasonably enforce anything about subclasses, except maybe warning if we see that we're adding an ivar (explicitly or via synthesis) to a class that's known to have a flexible array member.  Of course that means that the protection will be incomplete.

John.

Thanks everybody for their input. I plan to update the patch with different test cases so it's easier to discuss. But so far my summary is the following.

For flexible array members we allow them only as the last ivar in @interface, with no ivars in @implementation. If it's not last, if it is in @implementation, if there are more ivars in @implementation - that's an error.

For variable sized types (i.e. structs with last member a flexible array) we don't error but warn if we can detect more ivars after it (in @interface or in @implementation). If @implementation ivars end with variable sized ivar, no error or warning.

For subclassing we don't error but warn if we can detect more ivars after a flexible array member or after a variable sized type member.

I think most of this sounds reasonable.  I would recommend being lax about whether the ivar is declared in the @interface vs. the @implementation, though.

John.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
On 22 Sep 2017, at 06:31, John McCall <[hidden email]> wrote:

>
>> Thanks everybody for their input. I plan to update the patch with different test cases so it's easier to discuss. But so far my summary is the following.
>>
>> For flexible array members we allow them only as the last ivar in @interface, with no ivars in @implementation. If it's not last, if it is in @implementation, if there are more ivars in @implementation - that's an error.
>>
>> For variable sized types (i.e. structs with last member a flexible array) we don't error but warn if we can detect more ivars after it (in @interface or in @implementation). If @implementation ivars end with variable sized ivar, no error or warning.
>>
>> For subclassing we don't error but warn if we can detect more ivars after a flexible array member or after a variable sized type member.
>
> I think most of this sounds reasonable.  I would recommend being lax about whether the ivar is declared in the @interface vs. the @implementation, though.

If nothing else, I’d recommend a warning if the ivar is in the @implementation, because that means that we can’t guarantee getting a warning for subclasses, but apart from that this looks like a sensible design.  Thanks!

David


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev


> Le 21 sept. 2017 à 11:14, David Chisnall via cfe-dev <[hidden email]> a écrit :
>
> On 21 Sep 2017, at 10:07, Nat! via cfe-dev <[hidden email]> wrote:
>>
>> My second reaction, was to think that there are class cluster classes with allocate a flexible amount of memory (say string classes) that aren't ususally subclassed. Who am I to say, that using a convenient flexible array there can't be done (anymore) ?
>
> This is more or less how a number of the GNUstep Foundation classes work, using the extra data allocated with class_createInstance() and doing some pointer arithmetic.  Flexible array members would possibly simplify the code, but they’re not needed for this to work.
>
>> I would think a warning would be nice for classes who try to subclass a class with a flexible array ivar if they add another property or ivar.
>
> The problem is that you can’t do this with the non-fragile ABI.  You can write the flexible array member in the @implementation of a class and the person writing the subclass won’t see it.
>
> To make this safe, the minimum set of constraints that you’d need would be:
>
> - The ivar is the last one
> - The ivar is declared in the @interface
> - The class does not declare any ivars in the @implementation
> - Subclassing a class whose @interface ends with a flexible member is an error

And make the class fragile, which is probably not what you want it you use non-fragile ABI in the first place.

The goal of the non-fragile ABI is to be able to add ivar retroactively without breaking subclasses. That goal will not be possible with flexible array so they should not be allowed in anything but ‘final’ class IMHO.

If someone want to use that kind of risky trick, it should not use ivar in the first place, but override the +alloc method and perform pointer arithmetic to get a pointer on the extra storage. It will not be safer, but at least it will be explicit.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
On Sep 24, 2017, at 02:40, Jean-Daniel via cfe-dev <[hidden email]> wrote:

Le 21 sept. 2017 à 11:14, David Chisnall via cfe-dev <[hidden email]> a écrit :

On 21 Sep 2017, at 10:07, Nat! via cfe-dev <[hidden email]> wrote:

My second reaction, was to think that there are class cluster classes with allocate a flexible amount of memory (say string classes) that aren't ususally subclassed. Who am I to say, that using a convenient flexible array there can't be done (anymore) ?

This is more or less how a number of the GNUstep Foundation classes work, using the extra data allocated with class_createInstance() and doing some pointer arithmetic.  Flexible array members would possibly simplify the code, but they’re not needed for this to work.

I would think a warning would be nice for classes who try to subclass a class with a flexible array ivar if they add another property or ivar.

The problem is that you can’t do this with the non-fragile ABI.  You can write the flexible array member in the @implementation of a class and the person writing the subclass won’t see it.

To make this safe, the minimum set of constraints that you’d need would be:

- The ivar is the last one
- The ivar is declared in the @interface
- The class does not declare any ivars in the @implementation
- Subclassing a class whose @interface ends with a flexible member is an error

And make the class fragile, which is probably not what you want it you use non-fragile ABI in the first place.

The goal of the non-fragile ABI is to be able to add ivar retroactively without breaking subclasses. That goal will not be possible with flexible array so they should not be allowed in anything but ‘final’ class IMHO.

If someone want to use that kind of risky trick, it should not use ivar in the first place, but override the +alloc method and perform pointer arithmetic to get a pointer on the extra storage. It will not be safer, but at least it will be explicit.

The whole idea of flexible array members doesn’t work well with adding ivars retroactively. I am trying to warn about visible problems but it won’t catch all of them. And you still need to be extremely careful when mixing flexible array members and subclassing.

Volodymyr

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
On Sep 22, 2017, at 01:06, David Chisnall <[hidden email]> wrote:

On 22 Sep 2017, at 06:31, John McCall <[hidden email]> wrote:

Thanks everybody for their input. I plan to update the patch with different test cases so it's easier to discuss. But so far my summary is the following.

For flexible array members we allow them only as the last ivar in @interface, with no ivars in @implementation. If it's not last, if it is in @implementation, if there are more ivars in @implementation - that's an error.

For variable sized types (i.e. structs with last member a flexible array) we don't error but warn if we can detect more ivars after it (in @interface or in @implementation). If @implementation ivars end with variable sized ivar, no error or warning.

For subclassing we don't error but warn if we can detect more ivars after a flexible array member or after a variable sized type member.

I think most of this sounds reasonable.  I would recommend being lax about whether the ivar is declared in the @interface vs. the @implementation, though.

If nothing else, I’d recommend a warning if the ivar is in the @implementation, because that means that we can’t guarantee getting a warning for subclasses, but apart from that this looks like a sensible design.  Thanks!

David

Added test cases to https://reviews.llvm.org/D38114 Changes from my previous proposal:
* Allow flexible array members and variable sized types in @implementation but it should be the last field. And there is still a warning in this case.
* Error when variable sized type is not the last field (StructNotLastIVar, StructOtherIVarInImpl, StructNotLastIVarInImpl). Currently for C clang shows a warning that’s a GNU extension but by default GCC 7.2 rejects such code so I’m not sure it is worth supporting this extension for Objective-C.

Volodymyr

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Flexible array members in ObjC classes

Eric Fiselier via cfe-dev
On Sep 25, 2017, at 15:42, Volodymyr Sapsai via cfe-dev <[hidden email]> wrote:

On Sep 22, 2017, at 01:06, David Chisnall <[hidden email]> wrote:

On 22 Sep 2017, at 06:31, John McCall <[hidden email]> wrote:

Thanks everybody for their input. I plan to update the patch with different test cases so it's easier to discuss. But so far my summary is the following.

For flexible array members we allow them only as the last ivar in @interface, with no ivars in @implementation. If it's not last, if it is in @implementation, if there are more ivars in @implementation - that's an error.

For variable sized types (i.e. structs with last member a flexible array) we don't error but warn if we can detect more ivars after it (in @interface or in @implementation). If @implementation ivars end with variable sized ivar, no error or warning.

For subclassing we don't error but warn if we can detect more ivars after a flexible array member or after a variable sized type member.

I think most of this sounds reasonable.  I would recommend being lax about whether the ivar is declared in the @interface vs. the @implementation, though.

If nothing else, I’d recommend a warning if the ivar is in the @implementation, because that means that we can’t guarantee getting a warning for subclasses, but apart from that this looks like a sensible design.  Thanks!

David

Added test cases to https://reviews.llvm.org/D38114 Changes from my previous proposal:
* Allow flexible array members and variable sized types in @implementation but it should be the last field. And there is still a warning in this case.
* Error when variable sized type is not the last field (StructNotLastIVar, StructOtherIVarInImpl, StructNotLastIVarInImpl). Currently for C clang shows a warning that’s a GNU extension but by default GCC 7.2 rejects such code so I’m not sure it is worth supporting this extension for Objective-C.

Volodymyr

Those who are interested, can follow the code review at https://reviews.llvm.org/D38773

Volodymyr


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev