On 7/13/21 10:08 AM, Jonathan Wakely wrote:
On Mon, 12 Jul 2021 at 12:02, Richard Biener wrote:
Somebody with more C++ knowledge than me needs to approve the
vec.h changes - I don't feel competent to assess all effects of the change.
They look OK to me except for:
-extern vnull vNULL;
+static constexpr vnull vNULL{ };
Making vNULL have static linkage can make it an ODR violation to use
vNULL in templates and inline functions, because different
instantiations will refer to a different "vNULL" in each translation
unit.
The ODR says this is OK because it's a literal constant with the same
value (6.2/12.2.1).
But it would be better without the explicit 'static'; then in C++17 it's
implicitly inline instead of static.
But then, do we really want to keep vNULL at all? It's a weird blurring
of the object/pointer boundary that is also dependent on vec being a
thin wrapper around a pointer. In almost all cases it can be replaced
with {}; one exception is == comparison, where it seems to be testing
that the embedded pointer is null, which is a weird thing to want to test.
Somewhat relatedly, use of vec<T> variables or fields seems almost
always a mistake, as they need explicit .release() that could be
automatic with auto_vec, and is easy to forget. For instance, the
recursive call in get_all_loop_exits returns a vec that is never
released. And I see a couple of leaks in the C++ front end as well.
Jason
