diff mbox series

Explicitly document that the "counted_by" attribute is only supported in C

Message ID 20240805133301.1649647-1-qing.zhao@oracle.com
State New
Headers show
Series Explicitly document that the "counted_by" attribute is only supported in C | expand

Commit Message

Qing Zhao Aug. 5, 2024, 1:33 p.m. UTC 
As discussed in PR116016:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016#c48

We should explicitly document this limitation and issue error messages for C++.

The "counted_by" attribute currently is only supported in C, mention this
explicitly in documentation and also issue error when see "counted_by"
attribute in C++.

The patch has been bootstrappped and regression tested on both aarch64 and X86,
no issue.

Okay for committing?

thanks.

Qing

gcc/c-family/ChangeLog:

	* c-attribs.cc (handle_counted_by_attribute): Issue error for C++.

gcc/ChangeLog:

	* doc/extend.texi: Explicitly mentions counted_by is available
	only for C.

gcc/testsuite/ChangeLog:

	* g++.dg/flex-array-counted-by.C: New test.
---
 gcc/c-family/c-attribs.cc                    |  9 ++++++++-
 gcc/doc/extend.texi                          |  1 +
 gcc/testsuite/g++.dg/flex-array-counted-by.C | 11 +++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/flex-array-counted-by.C

Comments

Jakub Jelinek Aug. 5, 2024, 1:53 p.m. UTC | #1 
On Mon, Aug 05, 2024 at 01:33:01PM +0000, Qing Zhao wrote:
> As discussed in PR116016:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016#c48
> 
> We should explicitly document this limitation and issue error messages for C++.
> 
> The "counted_by" attribute currently is only supported in C, mention this
> explicitly in documentation and also issue error when see "counted_by"
> attribute in C++.
> 
> The patch has been bootstrappped and regression tested on both aarch64 and X86,
> no issue.
> 
> Okay for committing?
> 
> thanks.
> 
> Qing
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-attribs.cc (handle_counted_by_attribute): Issue error for C++.
> 
> gcc/ChangeLog:
> 
> 	* doc/extend.texi: Explicitly mentions counted_by is available
> 	only for C.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/flex-array-counted-by.C: New test.
> ---
>  gcc/c-family/c-attribs.cc                    |  9 ++++++++-
>  gcc/doc/extend.texi                          |  1 +
>  gcc/testsuite/g++.dg/flex-array-counted-by.C | 11 +++++++++++
>  3 files changed, 20 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/flex-array-counted-by.C
> 
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index 685f212683f..f936058800b 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -2859,8 +2859,15 @@ handle_counted_by_attribute (tree *node, tree name,
>    tree argval = TREE_VALUE (args);
>    tree old_counted_by = lookup_attribute ("counted_by", DECL_ATTRIBUTES (decl));
>  
> +  /* This attribute is not supported in C++.  */
> +  if (c_dialect_cxx ())
> +    {
> +      error_at (DECL_SOURCE_LOCATION (decl),
> +		"%qE attribute is not supported for C++", name);

This should be sorry_at instead IMHO (at least if there is a plan to support
it later, hopefully in the 15 timeframe).

> +      *no_add_attrs = true;
> +    }
>    /* This attribute only applies to field decls of a structure.  */
> -  if (TREE_CODE (decl) != FIELD_DECL)
> +  else if (TREE_CODE (decl) != FIELD_DECL)
>      {
>        error_at (DECL_SOURCE_LOCATION (decl),
>  		"%qE attribute is not allowed for a non-field"
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 48b27ff9f39..f31f3bdb53d 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -7848,6 +7848,7 @@ The @code{counted_by} attribute may be attached to the C99 flexible array
>  member of a structure.  It indicates that the number of the elements of the
>  array is given by the field "@var{count}" in the same structure as the
>  flexible array member.
> +This attribute is available only for C.

And this should say for now or something similar.

>  GCC may use this information to improve detection of object size information
>  for such structures and provide better results in compile-time diagnostics
>  and runtime features like the array bound sanitizer and
> diff --git a/gcc/testsuite/g++.dg/flex-array-counted-by.C b/gcc/testsuite/g++.dg/flex-array-counted-by.C
> new file mode 100644
> index 00000000000..7f1a345615e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/flex-array-counted-by.C

Tests shouldn't be added directly to g++.dg/ directory, I think this should
go into g++.dg/ext/ as it is an (unsupported) extension.

> @@ -0,0 +1,11 @@
> +/* Testing the fact that the attribute counted_by is not supported in C++.  */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +int size;
> +int x __attribute ((counted_by (size))); /* { dg-error "attribute is not supported for C\\+\\+" } */
> +
> +struct trailing {
> +  int count;
> +  int field[] __attribute ((counted_by (count))); /* { dg-error "attribute is not supported for C\\+\\+" } */
> +};

Maybe it should also test in another { dg-do compile { target c++11 } } test
that the same happens even for [[gnu::counted_by (size)]].
Seems even for C23 there are no tests with [[gnu::counted_by (size)]].
The C++11/C23 standard attributes are more strict on where they can appear
depending on what it appertains to, as it applies to declarations, I think
it needs to go before the [] or at the start of the declaration, so
  [[gnu::counted_by (count)]] int field[];
or
  int field [[gnu::counted_by (count)]] [];
but I could be wrong, better test it...

	Jakub
Qing Zhao Aug. 5, 2024, 4:46 p.m. UTC | #2 
> On Aug 5, 2024, at 09:53, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Mon, Aug 05, 2024 at 01:33:01PM +0000, Qing Zhao wrote:
>> As discussed in PR116016:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016#c48
>> 
>> We should explicitly document this limitation and issue error messages for C++.
>> 
>> The "counted_by" attribute currently is only supported in C, mention this
>> explicitly in documentation and also issue error when see "counted_by"
>> attribute in C++.
>> 
>> The patch has been bootstrappped and regression tested on both aarch64 and X86,
>> no issue.
>> 
>> Okay for committing?
>> 
>> thanks.
>> 
>> Qing
>> 
>> gcc/c-family/ChangeLog:
>> 
>> * c-attribs.cc (handle_counted_by_attribute): Issue error for C++.
>> 
>> gcc/ChangeLog:
>> 
>> * doc/extend.texi: Explicitly mentions counted_by is available
>> only for C.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> * g++.dg/flex-array-counted-by.C: New test.
>> ---
>> gcc/c-family/c-attribs.cc                    |  9 ++++++++-
>> gcc/doc/extend.texi                          |  1 +
>> gcc/testsuite/g++.dg/flex-array-counted-by.C | 11 +++++++++++
>> 3 files changed, 20 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/g++.dg/flex-array-counted-by.C
>> 
>> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
>> index 685f212683f..f936058800b 100644
>> --- a/gcc/c-family/c-attribs.cc
>> +++ b/gcc/c-family/c-attribs.cc
>> @@ -2859,8 +2859,15 @@ handle_counted_by_attribute (tree *node, tree name,
>>   tree argval = TREE_VALUE (args);
>>   tree old_counted_by = lookup_attribute ("counted_by", DECL_ATTRIBUTES (decl));
>> 
>> +  /* This attribute is not supported in C++.  */
>> +  if (c_dialect_cxx ())
>> +    {
>> +      error_at (DECL_SOURCE_LOCATION (decl),
>> + "%qE attribute is not supported for C++", name);
> 
> This should be sorry_at instead IMHO (at least if there is a plan to support
> it later, hopefully in the 15 timeframe).
Okay. 
> 
>> +      *no_add_attrs = true;
>> +    }
>>   /* This attribute only applies to field decls of a structure.  */
>> -  if (TREE_CODE (decl) != FIELD_DECL)
>> +  else if (TREE_CODE (decl) != FIELD_DECL)
>>     {
>>       error_at (DECL_SOURCE_LOCATION (decl),
>> "%qE attribute is not allowed for a non-field"
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 48b27ff9f39..f31f3bdb53d 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -7848,6 +7848,7 @@ The @code{counted_by} attribute may be attached to the C99 flexible array
>> member of a structure.  It indicates that the number of the elements of the
>> array is given by the field "@var{count}" in the same structure as the
>> flexible array member.
>> +This attribute is available only for C.
> 
> And this should say for now or something similar.
Okay. 
> 
> 
> 
>> GCC may use this information to improve detection of object size information
>> for such structures and provide better results in compile-time diagnostics
>> and runtime features like the array bound sanitizer and
>> diff --git a/gcc/testsuite/g++.dg/flex-array-counted-by.C b/gcc/testsuite/g++.dg/flex-array-counted-by.C
>> new file mode 100644
>> index 00000000000..7f1a345615e
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/flex-array-counted-by.C
> 
> Tests shouldn't be added directly to g++.dg/ directory, I think this should
> go into g++.dg/ext/ as it is an (unsupported) extension.
Makes sense.
> 
>> @@ -0,0 +1,11 @@
>> +/* Testing the fact that the attribute counted_by is not supported in C++.  */
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2" } */
>> +
>> +int size;
>> +int x __attribute ((counted_by (size))); /* { dg-error "attribute is not supported for C\\+\\+" } */
>> +
>> +struct trailing {
>> +  int count;
>> +  int field[] __attribute ((counted_by (count))); /* { dg-error "attribute is not supported for C\\+\\+" } */
>> +};
> 
> Maybe it should also test in another { dg-do compile { target c++11 } } test
> that the same happens even for [[gnu::counted_by (size)]].

Okay, will add one more test for c++11. 

> Seems even for C23 there are no tests with [[gnu::counted_by (size)]].
So, you want me to add counted_by test-suite for C23? (Which should be supported)
Okay, but I will do it in another separate patch since this patch is for C++. 
> The C++11/C23 standard attributes are more strict on where they can appear
> depending on what it appertains to, as it applies to declarations, I think
> it needs to go before the [] or at the start of the declaration, so
>  [[gnu::counted_by (count)]] int field[];
> or
>  int field [[gnu::counted_by (count)]] [];
> but I could be wrong, better test it…
For C++11, as I just checked:

 int field[] [[gnu::counted_by (count)]];

Is fine. 

thanks.

Qing
> 
> 
> Jakub
Jakub Jelinek Aug. 5, 2024, 4:51 p.m. UTC | #3 
On Mon, Aug 05, 2024 at 04:46:09PM +0000, Qing Zhao wrote:
> So, you want me to add counted_by test-suite for C23? (Which should be supported)
> Okay, but I will do it in another separate patch since this patch is for C++. 
> > The C++11/C23 standard attributes are more strict on where they can appear
> > depending on what it appertains to, as it applies to declarations, I think
> > it needs to go before the [] or at the start of the declaration, so
> >  [[gnu::counted_by (count)]] int field[];
> > or
> >  int field [[gnu::counted_by (count)]] [];
> > but I could be wrong, better test it…
> For C++11, as I just checked:
> 
>  int field[] [[gnu::counted_by (count)]];
> 
> Is fine. 

What do you mean by fine, that it emits the sorry?  Yes, but the question
is if it will be ok when the support is added.
struct S {
  int s;
  int f[] [[gnu::counted_by (s)]];
};
with -std=c23 certainly emits
test.c:3:3: warning: ‘counted_by’ attribute does not apply to types [-Wattributes]
    3 |   int f[] [[gnu::counted_by (s)]];
      |   ^~~
while it is fine for
  int f [[gnu::counted_by (s)]] [];
and
  [[gnu::counted_by (s)]] int f[];
So, I'd use that in the C++ testcase too...

	Jakub
Qing Zhao Aug. 5, 2024, 5:34 p.m. UTC | #4 
> On Aug 5, 2024, at 12:51, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Mon, Aug 05, 2024 at 04:46:09PM +0000, Qing Zhao wrote:
>> So, you want me to add counted_by test-suite for C23? (Which should be supported)
>> Okay, but I will do it in another separate patch since this patch is for C++.
>>> The C++11/C23 standard attributes are more strict on where they can appear
>>> depending on what it appertains to, as it applies to declarations, I think
>>> it needs to go before the [] or at the start of the declaration, so
>>> [[gnu::counted_by (count)]] int field[];
>>> or
>>> int field [[gnu::counted_by (count)]] [];
>>> but I could be wrong, better test it…
>> For C++11, as I just checked:
>> 
>> int field[] [[gnu::counted_by (count)]];
>> 
>> Is fine.
> 
> What do you mean by fine, that it emits the sorry?  Yes, but the question
> is if it will be ok when the support is added.
> struct S {
>  int s;
>  int f[] [[gnu::counted_by (s)]];
> };
> with -std=c23 certainly emits
> test.c:3:3: warning: ‘counted_by’ attribute does not apply to types [-Wattributes]
>    3 |   int f[] [[gnu::counted_by (s)]];
>      |   ^~~
> while it is fine for
>  int f [[gnu::counted_by (s)]] [];
> and
>  [[gnu::counted_by (s)]] int f[];
> So, I'd use that in the C++ testcase too...

Okay.

thanks.

Qing
> 
> 	Jakub
>
Jason Merrill Aug. 5, 2024, 5:48 p.m. UTC | #5 
On 8/5/24 9:53 AM, Jakub Jelinek wrote:
> On Mon, Aug 05, 2024 at 01:33:01PM +0000, Qing Zhao wrote:
>> As discussed in PR116016:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016#c48
>>
>> We should explicitly document this limitation and issue error messages for C++.
>>
>> The "counted_by" attribute currently is only supported in C, mention this
>> explicitly in documentation and also issue error when see "counted_by"
>> attribute in C++.
>>
>> The patch has been bootstrappped and regression tested on both aarch64 and X86,
>> no issue.
>>
>> +  /* This attribute is not supported in C++.  */
>> +  if (c_dialect_cxx ())
>> +    {
>> +      error_at (DECL_SOURCE_LOCATION (decl),
>> +		"%qE attribute is not supported for C++", name);
> 
> This should be sorry_at instead IMHO (at least if there is a plan to support
> it later, hopefully in the 15 timeframe).

Why should it be an error at all?  A warning seems sufficient since 
there's no semantic effect.

Jason
Jakub Jelinek Aug. 5, 2024, 5:54 p.m. UTC | #6 
On Mon, Aug 05, 2024 at 01:48:25PM -0400, Jason Merrill wrote:
> On 8/5/24 9:53 AM, Jakub Jelinek wrote:
> > On Mon, Aug 05, 2024 at 01:33:01PM +0000, Qing Zhao wrote:
> > > As discussed in PR116016:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016#c48
> > > 
> > > We should explicitly document this limitation and issue error messages for C++.
> > > 
> > > The "counted_by" attribute currently is only supported in C, mention this
> > > explicitly in documentation and also issue error when see "counted_by"
> > > attribute in C++.
> > > 
> > > The patch has been bootstrappped and regression tested on both aarch64 and X86,
> > > no issue.
> > > 
> > > +  /* This attribute is not supported in C++.  */
> > > +  if (c_dialect_cxx ())
> > > +    {
> > > +      error_at (DECL_SOURCE_LOCATION (decl),
> > > +		"%qE attribute is not supported for C++", name);
> > 
> > This should be sorry_at instead IMHO (at least if there is a plan to support
> > it later, hopefully in the 15 timeframe).
> 
> Why should it be an error at all?  A warning seems sufficient since there's
> no semantic effect.

Ok.  Guess OPT_Wattributes then.

	Jakub
Qing Zhao Aug. 5, 2024, 6:39 p.m. UTC | #7 
On Aug 5, 2024, at 13:54, Jakub Jelinek <jakub@redhat.com> wrote:

On Mon, Aug 05, 2024 at 01:48:25PM -0400, Jason Merrill wrote:
On 8/5/24 9:53 AM, Jakub Jelinek wrote:
On Mon, Aug 05, 2024 at 01:33:01PM +0000, Qing Zhao wrote:
As discussed in PR116016:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016#c48

We should explicitly document this limitation and issue error messages for C++.

The "counted_by" attribute currently is only supported in C, mention this
explicitly in documentation and also issue error when see "counted_by"
attribute in C++.

The patch has been bootstrappped and regression tested on both aarch64 and X86,
no issue.

+  /* This attribute is not supported in C++.  */
+  if (c_dialect_cxx ())
+    {
+      error_at (DECL_SOURCE_LOCATION (decl),
+ "%qE attribute is not supported for C++", name);

This should be sorry_at instead IMHO (at least if there is a plan to support
it later, hopefully in the 15 timeframe).

Why should it be an error at all?  A warning seems sufficient since there's
no semantic effect.

Ok.  Guess OPT_Wattributes then.

Okay.

Qing

Jakub
diff mbox series

Patch

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 685f212683f..f936058800b 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -2859,8 +2859,15 @@  handle_counted_by_attribute (tree *node, tree name,
   tree argval = TREE_VALUE (args);
   tree old_counted_by = lookup_attribute ("counted_by", DECL_ATTRIBUTES (decl));
 
+  /* This attribute is not supported in C++.  */
+  if (c_dialect_cxx ())
+    {
+      error_at (DECL_SOURCE_LOCATION (decl),
+		"%qE attribute is not supported for C++", name);
+      *no_add_attrs = true;
+    }
   /* This attribute only applies to field decls of a structure.  */
-  if (TREE_CODE (decl) != FIELD_DECL)
+  else if (TREE_CODE (decl) != FIELD_DECL)
     {
       error_at (DECL_SOURCE_LOCATION (decl),
 		"%qE attribute is not allowed for a non-field"
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 48b27ff9f39..f31f3bdb53d 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7848,6 +7848,7 @@  The @code{counted_by} attribute may be attached to the C99 flexible array
 member of a structure.  It indicates that the number of the elements of the
 array is given by the field "@var{count}" in the same structure as the
 flexible array member.
+This attribute is available only for C.
 GCC may use this information to improve detection of object size information
 for such structures and provide better results in compile-time diagnostics
 and runtime features like the array bound sanitizer and
diff --git a/gcc/testsuite/g++.dg/flex-array-counted-by.C b/gcc/testsuite/g++.dg/flex-array-counted-by.C
new file mode 100644
index 00000000000..7f1a345615e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/flex-array-counted-by.C
@@ -0,0 +1,11 @@ 
+/* Testing the fact that the attribute counted_by is not supported in C++.  */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int size;
+int x __attribute ((counted_by (size))); /* { dg-error "attribute is not supported for C\\+\\+" } */
+
+struct trailing {
+  int count;
+  int field[] __attribute ((counted_by (count))); /* { dg-error "attribute is not supported for C\\+\\+" } */
+};