Bug 29185 - inconsistent warning: deleting array
Summary: inconsistent warning: deleting array
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.1.0
: P3 enhancement
Target Milestone: 4.8.0
Assignee: Paolo Carlini
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2006-09-22 16:36 UTC by Martin Sebor
Modified: 2012-05-23 14:20 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 2.95.3, 3.0.4, 3.2.3, 3.3.3, 3.4.0, 4.0.0, 4.1.0, 4.2.0
Last reconfirmed: 2006-09-22 16:43:41


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2006-09-22 16:36:14 UTC
Gcc 4.1.0 warns about the (ill-fomed?) delete expression on line 5 but fails
to issue a diagnostic for the similar expressions on lines 6 and 7. I would
like to request that it consistently diagnose all such cases.

$ cat t.cpp && gcc -c t.cpp
/*   1 */ int a [1];
/*   2 */ struct S { int a [1]; } s;
/*   3 */ 
/*   4 */ void foo (S *p) {
/*   5 */     delete a;
/*   6 */     delete s.a;
/*   7 */     delete p->a;
/*   8 */ }
t.cpp: In function 'void foo(S*)':
t.cpp:5: warning: deleting array 'int a [1]'
Comment 1 Andrew Pinski 2006-09-22 16:43:41 UTC
> (ill-fomed?)
I think it is valid because of how arrays decay to pointers (EDG also accepts the code).

Confirmed, we just do the warning for array type decls:
  /* An array can't have been allocated by new, so complain.  */
  if (TREE_CODE (exp) == VAR_DECL
      && TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE)
    warning (0, "deleting array %q#D", exp);

Comment 2 Martin Sebor 2006-09-22 16:57:44 UTC
Yes, but 5.3.5, p1 says "The operand shall have a pointer type, or a class
type having a single conversion function (12.3.2) to a pointer type." and
not "shall be convertible to a pointer type." Note that gcc issues a hard
error for a dynamic_cast expression whose argument is an array, so I would
expect it to treat the delete expression the same since they both have the
same requirement WRT pointers.

Btw., I sent an email to EDG to request that they at least warn and to find
out whether they think it's well-formed. I'll update the incident with their
response.  Also note that both IBM XLC++ and HP aCC issue an error for the
test case.
Comment 3 Martin Sebor 2006-09-26 16:31:00 UTC
The response I got from EDG is that the expression is well-formed because of
5, p8. They do agree that issuing a warning would be useful and opened an
enhancement request.

FWIW, I think it should be ill-formed with diagnostic required since the
behavior of the expression is always undefined.
Comment 4 Andrew Pinski 2006-09-26 17:42:47 UTC
(In reply to comment #3)
> FWIW, I think it should be ill-formed with diagnostic required since the
> behavior of the expression is always undefined.

There are cases where this can show up in templates, someway or another.  And then undefined behavior at runtime cannot be rejected because someone it does not need to be run as someone could do:
if (0)
  delete a;
Comment 5 Martin Sebor 2006-09-26 18:56:42 UTC
You mean something like: if (is_pointer (p)) delete p;

I suppose that could happen but why should it be any different than other
non-sensical but lexically valid constructs with undefined behavior that
require a diagnostic today? E.g.:

    template <int N>
    void foo () {
        if (0 < N) {
            int array [N];
            ...
        }
    }

Or:

    template <class T, class U>
    U* bar (T *p) {
        if (is_convertible<T*, U*>)
            return p;
        return 0;
    }

Isn't template metaprogramming the expected solution to this type of a problem?
Comment 6 Andrew Pinski 2006-09-26 19:00:52 UTC
Subject: Re:  inconsistent warning: deleting array

> 
> 
> 
> ------- Comment #5 from sebor at roguewave dot com  2006-09-26 18:56 -------
> You mean something like: if (is_pointer (p)) delete p;
> 
> I suppose that could happen but why should it be any different than other
> non-sensical but lexically valid constructs with undefined behavior that
> require a diagnostic today? E.g.:
> 
>     template <int N>
>     void foo () {
>         if (0 < N) {
>             int array [N];
>             ...
>         }
>     }

That is not undefined behavior, just plain invalid.

> 
> Or:
> 
>     template <class T, class U>
>     U* bar (T *p) {
>         if (is_convertible<T*, U*>)
>             return p;
>         return 0;
>     }

Likewise.  This is a different issue.

> Isn't template metaprogramming the expected solution to this type of a problem?

int a[1];
int *b = a;
delete b;
is also undefined but it is hard to reject without having flow contrl inside the
front-end.


-- pinski
Comment 7 Martin Sebor 2006-09-26 21:43:32 UTC
You're right, those weren't the best examples, but I still think they
illustrate the point. The code in them is plain ill-formed even though
it never gets executed, because it just doesn't make sense. deleting
an array also doesn't make sense so it might as well be ill-formed.
I'll leave it to the committee to decide if that's a worthwhile change
to the language.
Comment 8 Martin Sebor 2006-09-28 16:16:18 UTC
The EDG guys don't think this is worth spending the committee's time on so I won't
be proposing any change to the standard. Issuing just a warning rather than an
error is good enough for me.

Also, I opened bug 29273 to remove the error from the dynamic_cast expression
with an array argument since that one is well-formed as well (see comment 2),
according to the same paragraph.
Comment 9 Paolo Carlini 2012-05-23 01:27:17 UTC
Mine.
Comment 10 paolo@gcc.gnu.org 2012-05-23 14:19:37 UTC
Author: paolo
Date: Wed May 23 14:19:27 2012
New Revision: 187801

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187801
Log:
/cp
2012-05-23  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/29185
	* decl2.c (delete_sanity): Extend 'deleting array' warning to
	any array type.

/testsuite
2012-05-23  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/29185
	* g++.dg/warn/delete-array-1.C: New.

Added:
    trunk/gcc/testsuite/g++.dg/warn/delete-array-1.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl2.c
    trunk/gcc/testsuite/ChangeLog
Comment 11 Paolo Carlini 2012-05-23 14:20:37 UTC
Done.