Bug 40192 - [4.4/4.5 Regression] Unable to use std::vector with typedef'd array types
Summary: [4.4/4.5 Regression] Unable to use std::vector with typedef'd array types
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.3.2
: P3 normal
Target Milestone: 4.4.1
Assignee: Paolo Carlini
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-18 21:06 UTC by Brian Cole
Modified: 2009-05-19 18:19 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-05-18 22:34:29


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Cole 2009-05-18 21:06:28 UTC
The following code will not compile in gcc 4.3.2 on Ubuntu 8.10

#include <vector>

typedef float float4[4];

int main()
{
  std::vector<float4> vals;
}

I get the following compilation error:
/usr/include/c++/4.3/bits/stl_construct.h: In function ‘void std::_Destroy(_Tp*) [with _Tp = float [4]]’:
/usr/include/c++/4.3/bits/stl_construct.h:103:   instantiated from ‘void std::_Destroy(_ForwardIterator, _ForwardIterator) [with _ForwardIterator = float (*)[4]]’
/usr/include/c++/4.3/bits/stl_construct.h:128:   instantiated from ‘void std::_Destroy(_ForwardIterator, _ForwardIterator, std::allocator<_T2>&) [with _ForwardIterator = float (*)[4], _Tp = float [4]]’
/usr/include/c++/4.3/bits/stl_vector.h:300:   instantiated from ‘std::vector<_Tp, _Alloc>::~vector() [with _Tp = float [4], _Alloc = std::allocator<float [4]>]’
test_float4.cpp:7:   instantiated from here
/usr/include/c++/4.3/bits/stl_construct.h:88: error: request for member ‘~float [4]’ in ‘* __pointer’, which is of non-class type ‘float [4]’

The code does compile in gcc 3.4 and gcc 4.1.
Comment 1 Paolo Carlini 2009-05-18 22:34:29 UTC
I'll fix it momentarily.
Comment 2 paolo@gcc.gnu.org 2009-05-18 23:16:38 UTC
Subject: Bug 40192

Author: paolo
Date: Mon May 18 23:16:20 2009
New Revision: 147680

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=147680
Log:
2009-05-18  Paolo Carlini  <paolo.carlini@oracle.com>

	PR libstdc++/40192
	* include/bits/stl_construct.h (struct _Destroy_aux): Add.
	(_Destroy(_ForwardIterator, _ForwardIterator)): Use the latter.
	* testsuite/23_containers/vector/40192.cc: New.


Added:
    trunk/libstdc++-v3/testsuite/23_containers/vector/40192.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/stl_construct.h

Comment 3 paolo@gcc.gnu.org 2009-05-18 23:17:12 UTC
Subject: Bug 40192

Author: paolo
Date: Mon May 18 23:16:48 2009
New Revision: 147681

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=147681
Log:
2009-05-18  Paolo Carlini  <paolo.carlini@oracle.com>

	PR libstdc++/40192
	* include/bits/stl_construct.h (struct _Destroy_aux): Add.
	(_Destroy(_ForwardIterator, _ForwardIterator)): Use the latter.
	* testsuite/23_containers/vector/40192.cc: New.


Added:
    branches/gcc-4_4-branch/libstdc++-v3/testsuite/23_containers/vector/40192.cc
Modified:
    branches/gcc-4_4-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_4-branch/libstdc++-v3/include/bits/stl_construct.h

Comment 4 Paolo Carlini 2009-05-18 23:18:28 UTC
Fixed for 4.4.1.
Comment 5 Jeff Schwab 2009-05-19 16:36:23 UTC
Whoa whoa whoa...  The behavior seemed correct before.  vector<float[4]> shouldn't even be legal.  Shouldn't the compiler to catch such a mistake?
Comment 6 Paolo Carlini 2009-05-19 16:42:17 UTC
I think that, in any case, we cannot do anything wrong here: we are simply restoring an implementation detail of the previous implementations, which, as a side effect, allows to compile this specific kind of snippet. Indeed, other implementations also swallow it. So, let's take the legal issues aside, and move on. If you figure specific details telling it's illegal just add it, as a note, but certainly diagnostic is not required.
Comment 7 Jeff Schwab 2009-05-19 17:09:18 UTC
I understand the desire for backward compatibility, but are the semantics actually the same?  Are the vector values arrays, or do they decay to pointers?  Section 23.1 says standard container elements have to be CopyConstructible and assignable, but raw arrays are neither.

Is there at least some flag to re-enable the diagnostic?  If you're saying this is a necessary evil for reasons of backward compatibility, then I understand, but in my opinion, this is a step backward that will confuse newcomers like Brian and hurt cross-compatibility.  Speaking strictly as a GCC user, I don't see any reason to reduce the compiler's ability to deduce an obvious mistake with ill-defined semantics.
Comment 8 Paolo Carlini 2009-05-19 17:12:43 UTC
This is not going to change again. We had this behaviour forever until 4.3.x and all the major implementations outside GCC behave the same as GCC now (and before the change of behavior).
Comment 9 Jeff Schwab 2009-05-19 17:32:57 UTC
OK.  Thanks for the explanation.  Are the semantics documented somewhere?
Comment 10 Paolo Carlini 2009-05-19 17:38:26 UTC
Look closely at the patch I committed, I changed *nothing* of the actual behavior of the containers. Thus the semantics is exactly the same as before, only, the destructor in not called explicitly anymore on such element type, because the destructor is trivial.
Comment 11 Brian Cole 2009-05-19 17:57:53 UTC
(In reply to comment #10)
> Look closely at the patch I committed, I changed *nothing* of the actual
> behavior of the containers. Thus the semantics is exactly the same as before,
> only, the destructor in not called explicitly anymore on such element type,
> because the destructor is trivial.
> 

Yup, even though you can declare a vector<float4>, you can't do anything useful with it. 

typedef float cl_float4[4] __attribute__((aligned(16)));

#include <vector>

int main()
{
  std::vector<cl_float4> vals(1);
  cl_float4 a = {1.0f, 2.0f, 3.0f, 4.0f};
  vals.push_back(a);
}

/usr/local/lib/gcc/x86_64-unknown-linux-gnu/4.1.2/../../../../include/c++/4.1.2/bits/stl_algobase.h:412: error: ISO C++ forbids assignment of arrays

Does seem strange to allow vector<float4> to be compilable, but eh, lesson learned, stay away from array types in C++. 
Comment 12 Jeff Schwab 2009-05-19 18:07:47 UTC
What he said.  I'm perusing your patch, and I appreciate that you removed an artificial restriction.  The right place to catch this is up front, in a concept check, rather than in _Destroy; since I'm not about to add the check myself, I'm hardly in a position to criticize.  The diagnostic did have the nice property of catching a real, semantic error, though, and it seems a shame to let a known error go unreported.
Comment 13 Paolo Carlini 2009-05-19 18:19:04 UTC
It's life, you know ;)