Boost C++ Libraries: Ticket #3472: Setting value_initialized to a value when T is a top-level const

owner changed; cc set

niels_dekker — Tue, 22 Sep 2009 05:19:26 GMT

cc niels_address_until_2010-10-10@… added
owner changed from No-Maintainer to Fernando Cacciola

Related discussion starts here: [Boost-users] [value_initialized] when T is const, http://lists.boost.org/boost-users/2009/09/52139.php

attachment set

Wed, 23 Sep 2009 18:27:29 GMT

attachment → value_init.patch

Patch to latest SVN implementing fix

niels_dekker — Tue, 05 Jan 2010 21:56:30 GMT

I still think your request is very reasonable. So I have just had another look at your proposed patch, which adds an explicit value_initialized(T const&) constructor. But now I'm a little bit worried that the patch might cause some confusion or ambiguity between your proposed constructor and the existing copy-constructor of value_initialized. You know, a constructor with a single parameter could get some compiler confused, when generating an implementation-defined copy-constructor, as you have have recently reported to Microsoft. Moreover, I believe there might be use cases where ambiguity would occur, even without a compiler bug.

So wouldn't it be wiser to add an extra parameter to your constructor, as a tag (similar to std::allocator_arg_t in the latest C++0x Draft, section [memory]). Such a parameter might unambiguously specify that your constructor would direct-initialize the object, instead of doing value-initialization. I would propose to call the tag type boost::direct_initialized_t, and add the following lines to value_init.hpp:

  struct direct_initialized_t { };
  const direct_initialized_t direct_initialized = direct_initialized_t();

Your constructor could then be defined as follows:

   explicit value_initialized(T const & arg, direct_initialized_t)
   {
     new (wrapper_address()) wrapper(arg);
   }

What do you think?

Wed, 06 Jan 2010 01:11:19 GMT

Replying to niels_dekker:

I still think your request is very reasonable. So I have just had another look at your proposed patch, which adds an explicit value_initialized(T const&) constructor. But now I'm a little bit worried that the patch might cause some confusion or ambiguity between your proposed constructor and the existing copy-constructor of value_initialized. You know, a constructor with a single parameter could get some compiler confused, when generating an implementation-defined copy-constructor, as you have have recently reported to Microsoft. Moreover, I believe there might be use cases where ambiguity would occur, even without a compiler bug.

So wouldn't it be wiser to add an extra parameter to your constructor, as a tag (similar to std::allocator_arg_t in the latest C++0x Draft, section [memory]). Such a parameter might unambiguously specify that your constructor would direct-initialize the object, instead of doing value-initialization. I would propose to call the tag type boost::direct_initialized_t, and add the following lines to value_init.hpp:
  struct direct_initialized_t { };
  const direct_initialized_t direct_initialized = direct_initialized_t();
Your constructor could then be defined as follows:
   explicit value_initialized(T const & arg, direct_initialized_t)
   {
     new (wrapper_address()) wrapper(arg);
   }
What do you think?

I think that doing such contortions to avoid a Microsoft bug is unwise. You are basically saying, as I see it, that adding the 'explicit value_initialized(T const & arg)' along with the already existing copy constructor of 'value_initialized(value_initialized const & arg)' is going to cause VC++ to call the wrong constructor when trying to copy a value-initialized value. As I understand the Microsoft bug the problem lies when Microsoft generates a user-defined copy constructor in a derived class and this calls down to the incorrect base class constructor. Unless someone is going to derive a class from value_initialized, which is probably not going to be done very often, my added constructor will not cause problems with VC++. Even when it does in that particular case, do you really believe that it should be necessary to contort C++ just to deal with a single compiler's problem ? I would much rather have the person who might consider deriving from value_initialized and who uses VC++ to be made aware of the VC++ issue, possibly in comments in the value_initialized code, and to use the workaround to get around this bug of defining one's own derived class copy constructor that calls the base class's copy constructor.

I strongly feel it is really a bad idea to change or complicate normal design and coding because of the an obvious bug in a particular implementation of a computer language. Please note I consider this as different from a compiler that simply doesn't support some feature of a language, where workarounds may be created for that compiler, so that one can implement some library design in another often less proficient and natural way. Not implementing a templated constructor in a perfectly normal way as opposed to a completely unusual and much harder to use choice can not be right.

niels_dekker — Wed, 06 Jan 2010 09:51:44 GMT

Hi Edward, I understand you do not want to adapt your proposed constructor, merely because of a Microsoft specific compiler bug. But I think your patch might break some use cases on other compilers as well:

class my_integer
{
  value_initialized<int> m_data;
public:
  operator value_initialized<int>() const;
  operator int() const;
};
int main()
{
  my_integer my;
  value_initialized<int> val(my);
}

The above example will become ambiguous when your proposed constructor is added, also according to GCC 4.1.2: http://codepad.org/zukxSDbB

I think there's a trade off between safety and convenience here. BTW, why did you add an explicit keyword to your constructor?

Thu, 07 Jan 2010 05:22:17 GMT

Replying to niels_dekker:

Hi Edward, I understand you do not want to adapt your proposed constructor, merely because of a Microsoft specific compiler bug. But I think your patch might break some use cases on other compilers as well:
class my_integer
{
  value_initialized<int> m_data;
public:
  operator value_initialized<int>() const;
  operator int() const;
};
int main()
{
  my_integer my;
  value_initialized<int> val(my);
}
The above example will become ambiguous when your proposed constructor is added, also according to GCC 4.1.2: http://codepad.org/zukxSDbB

I think there's a trade off between safety and convenience here. BTW, why did you add an explicit keyword to your constructor?

It sounds to me that you are rejecting any class that has a constructor which takes a single parameter because it may lead to an ambiguous situation like the one you show above. That seems to me to be unnecessarily restrictive and would eliminate much of C++ class design as it has previously been practiced. Consider:

struct X { X() {} X(const X & value) {} X(int value) {} };

struct Y { Y():anInt(5) {} operator X() const {return anX;} operator int() const {return anInt} private: X anX; int anInt; };

int main() { Y aY; X anotherX(ay); ambiguity X YetAnotherX(static_cast<X>(ay)); eliminates ambiguity X YetAgainX(static_cast<int>(ay)); eliminates ambiguity }

One can create this situation with any class, like X, which has a constructor taking a single value. So I still do not see the point of yout objection to this very common usage in value_initialized. I know, or I think I know, that you can't seriously be suggesting that all classes with any constructors which take a single parameter are badly designed, and therefore need to add a second dummy parameter in each case just to avoid the possibly ambiguity as ou showed and I have shown above.

niels_dekker — Thu, 07 Jan 2010 18:46:44 GMT

Replying to Edward Diener:

It sounds to me that you are rejecting any class that has a constructor which takes a single parameter because it may lead to an ambiguous situation like the one you show above.

I am not rejecting any class that has a constructor which takes a single parameter. Although I have to admit, both that particular Visual C++ bug report of yours and CWG issue 535, "Copy construction without a copy constructor" have made me more aware that such a constructor may cause confusion or ambiguity.

But I do think value_initialized should be dealt with with extra care. Because it is around now for more than seven years, and I assume it is used in quite a lot of programs worldwide. Thereby value_initialized is by its nature very generic, so it's very well possible that your proposed change might break some user code in an unexpected way. Moreover, your proposed constructor offers a loophole, which allows to break the class invariant, being that value_initialized holds an object that has been value-initialized.

Now it might in some cases be acceptable to break backward compatibility. And I think that there are very good reasons to allow storing a direct-initialized object into a value_initialized wrapper. But I just think the considerations above have to be taken into account when deciding how to resolve your value_init request.

I'm certainly not against your proposed patch, it's just that I would prefer to add a tag parameter (for example, boost::direct_initialized) to your constructor. But it appears that you're strongly opposed to that idea of mine, right?

Anyway, I just hope Fernando can make the right choice :-)

niels_dekker — Wed, 31 Mar 2010 14:37:12 GMT

Jeffrey Hellrung suggested defining a new template class that offers both value-initialization and direct-initialization: Re: [boost][utility/value_init] boost::value_initialized<T> direct-initialized?, http://lists.boost.org/Archives/boost/2010/03/164331.php The new template class could simply be named boost::initialized<T>. value_initialized<T> could then be implemented in terms of initialized<T>.

attachment set

niels_dekker — Sat, 03 Apr 2010 12:23:58 GMT

attachment → initialized.patch

Adds a new template class to value_init.hpp: boost::initialized<T>

attachment set

niels_dekker — Sat, 01 May 2010 22:04:58 GMT

attachment → initialized_documentation.patch

documentation for boost::initialized<T>

attachment set

niels_dekker — Sun, 02 May 2010 09:52:21 GMT

attachment → initialized_test.patch

tests for boost::initialized<T> (fixed a few typo's)

niels_dekker — Sun, 09 May 2010 20:51:31 GMT

(In [61883]) Added boost::initialized<T> as was agreed at http://lists.boost.org/Archives/boost/2010/04/164916.php -- see #3472

niels_dekker — Sun, 04 Jul 2010 21:56:49 GMT

(In [63638]) Merged value_init fixes (extra tests + documentation) from trunk, see #3472, #3869.

status changed; resolution set

niels_dekker — Thu, 15 Jul 2010 10:25:42 GMT

status new → closed
resolution → fixed

Fixed for Boost release 1.44, by having added boost::initialized<T>.