On Wed, 14 Jul 2021 at 04:39, Jason Merrill <ja...@redhat.com> wrote:
>
> On 7/13/21 4:02 PM, Martin Sebor wrote:
> > On 7/13/21 12:37 PM, Jason Merrill wrote:
> >> 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.
> >
> > I'll remove the 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.
> >
> > The one use case I know of for vNULL where I can't think of
> > an equally good substitute is in passing a vec as an argument by
> > value. The only way to do that that I can think of is to name
> > the full vec type (i.e., the specialization) which is more typing
> > and less generic than vNULL. I don't use vNULL myself so I wouldn't
> > miss this trick if it were to be removed but others might feel
> > differently.
>
> In C++11, it can be replaced by {} in that context as well.
Or if people don't like that, you could add a constructor taking
std::nullptr_t and an equality comparison with std::nullptr_t and then
use nullptr instead of vNULL.
I think just using {} for an empty, value-initialized vec makes more
sense though.
