> On Mar 10, 2023, at 2:54 AM, Richard Biener <rguent...@suse.de> wrote:
> 
> On Thu, 9 Mar 2023, Qing Zhao wrote:
> 
>> 
>> 
>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguent...@suse.de> wrote:
>>> 
>>> On Fri, 24 Feb 2023, Qing Zhao wrote:
>>> 
>>>> GCC extension accepts the case when a struct with a C99 flexible array 
>>>> member
>>>> is embedded into another struct or union (possibly recursively).
>>>> __builtin_object_size should treat such struct as flexible size.
>>>> 
>>>> gcc/c/ChangeLog:
>>>> 
>>>>    PR tree-optimization/101832
>>>>    * c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>>>>    struct/union type.
>>> 
>>> I can't really comment on the correctness of this part but since
>>> only the C frontend will ever set this and you are using it from
>>> addr_object_size which is also used for other C family languages
>>> (at least), I wonder if you can really test
>>> 
>>> +                       if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>> 
>>> there.
>> 
>> You mean for C++ and also other C family languages (other than C), the above 
>> bit cannot be set?
>> Yes, that’s true. The bit is only set for C. So is the bit 
>> DECL_NOT_FLEXARRAY, which is only set for C too. 
>> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in 
>> middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can 
>> set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
>> What’s your suggestion?
>> 
>> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should 
>> be set in the same places).
> 
> I was wondering if the above test errors on the conservative side
> correctly - it will now, for all but C, cut off some thing where it
> didn't before?

As long as the default value of TYPE_INCLUDE_FLEXARRAY reflects the correct 
conservative  behavior, then the testing should be correct, right?

The default value of TYPE_INCLUDE_FLEXARRAY is 0, i.e, FALSE, means that the 
TYPE does not include a flex array by default, this is the correct behavior. 
Only when the TYPE does include a flexiarray, it will be set to TRUE. So, I 
think it’s correct.

This is a different situation for DECL_NOT_FLEXARRAY, by default, the compiler 
will treat ALL trailing arrays as FMA, so in order to keep the correct 
conservative behavior, we should keep the default value for DECL_NOT_FLEXARRAY 
as it’s a FMA, i.e, DECL_NOT_FLEXARRAY being FALSE, by default. Only when the 
array is NOT a FMA, we set it to true. 

So, the default value for TYPE_INCLUDE_FLEXARRAY is correct. 
> 
>>> 
>>> Originally I was suggesting to set this flag in stor-layout.cc
>>> which eventually all languages funnel their types through and
>>> if there's language specific handling use a langhook (with the
>>> default implementation preserving the status quo).
>> 
>> If we decide to set the bits in stor-layout.cc, where is the best place to 
>> do it? I checked the star-layout.cc code, looks like “layout_type” might be 
>> the place where we can set these bits for RECORD_TYPE, UNION_TYPE? 
> 
> Yes, it would be layout_type.
> 
>>> 
>>> Some more comments below ...
>>> 
>>>> gcc/cp/ChangeLog:
>>>> 
>>>>    PR tree-optimization/101832
>>>>    * module.cc (trees_out::core_bools): Stream out new bit
>>>>    type_include_flexarray.
>>>>    (trees_in::core_bools): Stream in new bit type_include_flexarray.
>>>> 
>>>> gcc/ChangeLog:
>>>> 
>>>>    PR tree-optimization/101832
>>>>    * print-tree.cc (print_node): Print new bit type_include_flexarray.
>>>>    * tree-core.h (struct tree_type_common): New bit
>>>>    type_include_flexarray.
>>>>    * tree-object-size.cc (addr_object_size): Handle structure/union type
>>>>    when it has flexible size.
>>>>    * tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
>>>>    in new bit type_include_flexarray.
>>>>    * tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
>>>>    out new bit type_include_flexarray.
>>>>    * tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
>>>>    TYPE_INCLUDE_FLEXARRAY.
>>>> 
>>>> gcc/testsuite/ChangeLog:
>>>> 
>>>>    PR tree-optimization/101832
>>>>    * gcc.dg/builtin-object-size-pr101832.c: New test.
>>>> ---
>>>> gcc/c/c-decl.cc                               |  12 ++
>>>> gcc/cp/module.cc                              |   2 +
>>>> gcc/print-tree.cc                             |   5 +
>>>> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
>>>> gcc/tree-core.h                               |   4 +-
>>>> gcc/tree-object-size.cc                       |  79 +++++++----
>>>> gcc/tree-streamer-in.cc                       |   1 +
>>>> gcc/tree-streamer-out.cc                      |   1 +
>>>> gcc/tree.h                                    |   6 +
>>>> 9 files changed, 215 insertions(+), 29 deletions(-)
>>>> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>> 
>>>> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
>>>> index 08078eadeb8..f589a2f5192 100644
>>>> --- a/gcc/c/c-decl.cc
>>>> +++ b/gcc/c/c-decl.cc
>>>> @@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree 
>>>> fieldlist, tree attributes,
>>>>      /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
>>>>      DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, 
>>>> x);
>>>> 
>>>> +      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>>>> +       * when x is an array.  */
>>>> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
>>>> +  TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE 
>>>> (x)) ;
>>>> +      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>>>> +   when x is the last field.  */
>>>> +      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
>>>> +          || TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
>>>> +         && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
>>>> +         && is_last_field)
>>>> +  TYPE_INCLUDE_FLEXARRAY (t) = true;
>>>> +
>>>>      if (DECL_NAME (x)
>>>>      || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
>>>>    saw_named_field = true;
>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>>>> index ac2fe66b080..c750361b704 100644
>>>> --- a/gcc/cp/module.cc
>>>> +++ b/gcc/cp/module.cc
>>>> @@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
>>>>      WB (t->type_common.lang_flag_5);
>>>>      WB (t->type_common.lang_flag_6);
>>>>      WB (t->type_common.typeless_storage);
>>>> +      WB (t->type_common.type_include_flexarray);
>>>>    }
>>>> 
>>>>  if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>>>> @@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
>>>>      RB (t->type_common.lang_flag_5);
>>>>      RB (t->type_common.lang_flag_6);
>>>>      RB (t->type_common.typeless_storage);
>>>> +      RB (t->type_common.type_include_flexarray);
>>>>    }
>>>> 
>>>>  if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>>>> diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
>>>> index 1f3afcbbc86..efacdb7686f 100644
>>>> --- a/gcc/print-tree.cc
>>>> +++ b/gcc/print-tree.cc
>>>> @@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree 
>>>> node, int indent,
>>>>      && TYPE_CXX_ODR_P (node))
>>>>    fputs (" cxx-odr-p", file);
>>>> 
>>>> +      if ((code == RECORD_TYPE
>>>> +     || code == UNION_TYPE)
>>>> +    && TYPE_INCLUDE_FLEXARRAY (node))
>>>> +  fputs (" include-flexarray", file);
>>>> +
>>>>      /* The transparent-union flag is used for different things in
>>>>     different nodes.  */
>>>>      if ((code == UNION_TYPE || code == RECORD_TYPE)
>>>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c 
>>>> b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>> new file mode 100644
>>>> index 00000000000..60078e11634
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>> @@ -0,0 +1,134 @@
>>>> +/* PR 101832: 
>>>> +   GCC extension accepts the case when a struct with a C99 flexible array
>>>> +   member is embedded into another struct (possibly recursively).
>>>> +   __builtin_object_size will treat such struct as flexible size.
>>>> +   However, when a structure with non-C99 flexible array member, i.e, 
>>>> trailing
>>>> +   [0], [1], or [4], is embedded into anther struct, the stucture will not
>>>> +   be treated as flexible size.  */ 
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-O2" } */
>>>> +
>>>> +#include "builtin-object-size-common.h"
>>>> +
>>>> +#define expect(p, _v) do { \
>>>> +  size_t v = _v; \
>>>> +  if (p == v) \
>>>> +    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
>>>> +  else {\
>>>> +    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
>>>> +    FAIL (); \
>>>> +  } \
>>>> +} while (0);
>>>> +
>>>> +
>>>> +struct A {
>>>> +  int n;
>>>> +  char data[];
>>>> +};
>>>> +
>>>> +struct B {
>>>> +  int m;
>>>> +  struct A a;
>>>> +};
>>>> +
>>>> +struct C {
>>>> +  int q;
>>>> +  struct B b;
>>>> +};
>>>> +
>>>> +struct A0 {
>>>> +  int n;
>>>> +  char data[0];
>>>> +};
>>>> +
>>>> +struct B0 {
>>>> +  int m;
>>>> +  struct A0 a;
>>>> +};
>>>> +
>>>> +struct C0 {
>>>> +  int q;
>>>> +  struct B0 b;
>>>> +};
>>>> +
>>>> +struct A1 {
>>>> +  int n;
>>>> +  char data[1];
>>>> +};
>>>> +
>>>> +struct B1 {
>>>> +  int m;
>>>> +  struct A1 a;
>>>> +};
>>>> +
>>>> +struct C1 {
>>>> +  int q;
>>>> +  struct B1 b;
>>>> +};
>>>> +
>>>> +struct An {
>>>> +  int n;
>>>> +  char data[8];
>>>> +};
>>>> +
>>>> +struct Bn {
>>>> +  int m;
>>>> +  struct An a;
>>>> +};
>>>> +
>>>> +struct Cn {
>>>> +  int q;
>>>> +  struct Bn b;
>>>> +};
>>>> +
>>>> +volatile void *magic1, *magic2;
>>>> +
>>>> +int main (int argc, char *argv[])
>>>> +{
>>>> +  struct B *outer;
>>>> +  struct C *outest;
>>>> +
>>>> +  /* Make sure optimization can't find some other object size. */
>>>> +  outer = (void *)magic1;
>>>> +  outest = (void *)magic2;
>>>> +
>>>> +  expect (__builtin_object_size (&outer->a, 1), -1);
>>>> +  expect (__builtin_object_size (&outest->b, 1), -1);
>>>> +  expect (__builtin_object_size (&outest->b.a, 1), -1);
>>>> +
>>>> +  struct B0 *outer0;
>>>> +  struct C0 *outest0;
>>>> +
>>>> +  /* Make sure optimization can't find some other object size. */
>>>> +  outer0 = (void *)magic1;
>>>> +  outest0 = (void *)magic2;
>>>> +
>>>> +  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
>>>> +  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
>>>> +  expect (__builtin_object_size (&outest0->b.a, 1), sizeof 
>>>> (outest0->b.a));
>>>> +
>>>> +  struct B1 *outer1;
>>>> +  struct C1 *outest1;
>>>> +
>>>> +  /* Make sure optimization can't find some other object size. */
>>>> +  outer1 = (void *)magic1;
>>>> +  outest1 = (void *)magic2;
>>>> +
>>>> +  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
>>>> +  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
>>>> +  expect (__builtin_object_size (&outest1->b.a, 1), sizeof 
>>>> (outest1->b.a));
>>>> +
>>>> +  struct Bn *outern;
>>>> +  struct Cn *outestn;
>>>> +
>>>> +  /* Make sure optimization can't find some other object size. */
>>>> +  outern = (void *)magic1;
>>>> +  outestn = (void *)magic2;
>>>> +
>>>> +  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
>>>> +  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
>>>> +  expect (__builtin_object_size (&outestn->b.a, 1), sizeof 
>>>> (outestn->b.a));
>>>> +
>>>> +  DONE ();
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
>>>> index acd8deea34e..705d5702b9c 100644
>>>> --- a/gcc/tree-core.h
>>>> +++ b/gcc/tree-core.h
>>>> @@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
>>>>  unsigned empty_flag : 1;
>>>>  unsigned indivisible_p : 1;
>>>> RECORD_OR_UNION_CHECK
>>> 
>>> it looks like the bit could be eventually shared with
>>> no_named_args_stdarg_p (but its accessor doesn't use
>>> FUNC_OR_METHOD_CHECK)
>> You mean to let “type_include_flexarray” share the same bit with 
>> “no_named_args_stdarg_p” in order to save 1-bit space in IR?
>> Then change the 
>> 
>> /* True if this is a stdarg function with no named arguments (C2x
>>   (...) prototype, where arguments can be accessed with va_start and
>>   va_arg), as opposed to an unprototyped function.  */
>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>  (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>> 
>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>   at the last field recursively.  */
>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
>> 
>> To:
>> 
>> union {
>>    unsigned no_named_args_stdarg_p : 1;
>>    /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
>>    unsigned int type_include_flexarray : 1;
>>  } shared_bit;
>>  unsigned spare : 15;
>> 
>> /* True if this is a stdarg function with no named arguments (C2x
>>   (...) prototype, where arguments can be accessed with va_start and
>>   va_arg), as opposed to an unprototyped function.  */
>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>  (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.no_named_args_stdarg_p)
>> 
>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>   at the last field recursively.  */
>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>  (RECORD_OR_UNION_CHECK(NODE)->type_common.shared_bit.type_include_flexarray)
> 
> No, we're just overloading bits by using the same name for different
> purposes.  I don't think such a union would pack nicely.  We overload
> by documenting the uses, in the above cases the uses are separated
> by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
> and the accessor macros should be updated to use the appropriate
> tree check macros covering that so we don't try to inspect the
> "wrong bit”.
Okay, I see. Then should I change the name of that bit to one that reflect two 
usages?
> 
>>> 
>>>>  alias_set_type alias_set;
>>>>  tree pointer_to;
>>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>>>> index 9a936a91983..22b3c72ea6e 100644
>>>> --- a/gcc/tree-object-size.cc
>>>> +++ b/gcc/tree-object-size.cc
>>>> @@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, 
>>>> const_tree ptr,
>>>>                v = NULL_TREE;
>>>>                break;
>>>>              case COMPONENT_REF:
>>>> -              if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>>>> +              /* When the ref is not to an array, a record or a union, it
>>>> +                 will not have flexible size, compute the object size
>>>> +                 directly.  */
>>>> +              if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>>>> +                  && (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
>>>> +                  && (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
>>>>                  {
>>>>                    v = NULL_TREE;
>>>>                    break;
>>>>                  }
>>>> -              is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
>>>> -              while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>> -                if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -                    != UNION_TYPE
>>>> -                    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -                    != QUAL_UNION_TYPE)
>>>> -                  break;
>>>> -                else
>>>> -                  v = TREE_OPERAND (v, 0);
>>>> -              if (TREE_CODE (v) == COMPONENT_REF
>>>> -                  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -                     == RECORD_TYPE)
>>>> +              if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
>>>> +                  || TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
>>>> +              /* if the record or union does not include a flexible array
>>>> +                 recursively, compute the object size directly.  */
>>>>                  {
>>>> -                  /* compute object size only if v is not a
>>>> -                     flexible array member.  */
>>>> -                  if (!is_flexible_array_mem_ref)
>>>> +                  if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>>>                      {
>>>>                        v = NULL_TREE;
>>>>                        break;
>>>>                      }
>>>> -                  v = TREE_OPERAND (v, 0);
>>>> +                  else
>>>> +                    v = TREE_OPERAND (v, 0);
>>>>                  }
>>>> -              while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>> -                if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -                    != UNION_TYPE
>>>> -                    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -                    != QUAL_UNION_TYPE)
>>>> -                  break;
>>>> -                else
>>>> -                  v = TREE_OPERAND (v, 0);
>>>> -              if (v != pt_var)
>>>> -                v = NULL_TREE;
>>>>                else
>>>> -                v = pt_var;
>>>> +                {
>>>> +                  /* Now the ref is to an array type.  */
>>>> +                  is_flexible_array_mem_ref
>>>> +                    = array_ref_flexible_size_p (v);
>>>> +                  while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>> +                  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +                        != UNION_TYPE
>>>> +                      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +                           != QUAL_UNION_TYPE)
>>>> +                    break;
>>>> +                  else
>>>> +                    v = TREE_OPERAND (v, 0);
>>>> +                  if (TREE_CODE (v) == COMPONENT_REF
>>>> +                      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +                           == RECORD_TYPE)
>>>> +                    {
>>>> +                      /* compute object size only if v is not a
>>>> +                         flexible array member.  */
>>>> +                      if (!is_flexible_array_mem_ref)
>>>> +                        {
>>>> +                          v = NULL_TREE;
>>>> +                          break;
>>>> +                        }
>>>> +                      v = TREE_OPERAND (v, 0);
>>>> +                    }
>>>> +                  while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>> +                    if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +                          != UNION_TYPE
>>>> +                        && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +                             != QUAL_UNION_TYPE)
>>>> +                      break;
>>>> +                    else
>>>> +                      v = TREE_OPERAND (v, 0);
>>>> +                  if (v != pt_var)
>>>> +                    v = NULL_TREE;
>>>> +                  else
>>>> +                    v = pt_var;
>>>> +                }
>>>>                break;
>>>>              default:
>>>>                v = pt_var;
>>>> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
>>>> index d4dc30f048f..c19ede0631d 100644
>>>> --- a/gcc/tree-streamer-in.cc
>>>> +++ b/gcc/tree-streamer-in.cc
>>>> @@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d 
>>>> *bp, tree expr)
>>>>      TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>      TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>      TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>> +      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>    }
>>>>  else if (TREE_CODE (expr) == ARRAY_TYPE)
>>>>    TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
>>>> index d107229da5c..73e4b4e547c 100644
>>>> --- a/gcc/tree-streamer-out.cc
>>>> +++ b/gcc/tree-streamer-out.cc
>>>> @@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d 
>>>> *bp, tree expr)
>>>>      bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
>>>>                     ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
>>>>                     : TYPE_CXX_ODR_P (expr), 1);
>>>> +      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
>>>>    }
>>>>  else if (TREE_CODE (expr) == ARRAY_TYPE)
>>>>    bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
>>>> diff --git a/gcc/tree.h b/gcc/tree.h
>>>> index 92ac0e6a214..ab1cdc3dc85 100644
>>>> --- a/gcc/tree.h
>>>> +++ b/gcc/tree.h
>>>> @@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed 
>>>> (const_tree, const char *, int,
>>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>>>  (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>>> 
>>>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>>> +   at the last field recursively.  */
>>>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>>> +  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
>>> 
>>> Please use RECORD_OR_UNION_CHECK.  
>> Okay. 
>>> The comment "at the last field"
>>> doesn't seem to match implementation? (at least not obviously)
>> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the 
>> last field. (And recursively). 
>>> Given this is a generic header expanding on what a "flexible array member"
>>> is to the middle-end here would be good.  Especially the guarantees
>>> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
>>> vs. DECL_FLEXARRAY).
>> 
>> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex 
>> array by default.
>> It’s only set to TRUE when the struct/union includes a flex array at the 
>> last field (and recursively). So, I think it should be correct. 
>> Let me know if I missed anything here.
> 
> See above - I was looking at the use (but I'm not familiar with that 
> code), and wondered how the change affects uses from other frontends.

Please see my explanation above, I think the default behavior from other FE 
should be kept correctly with this patch.

thanks.

Qing
> 
> Richard.

Reply via email to