Modify

Ticket #3472 (closed Feature Requests: fixed)

Opened 5 years ago

Last modified 4 years ago

Setting value_initialized<T> to a value when T is a top-level const

Reported by: Edward Diener <eld@…> Owned by: fcacciola
Milestone: Boost 1.41.0 Component: utility
Version: Boost 1.40.0 Severity: Problem
Keywords: value_initialized const Cc: niels_address_until_2010-10-10@…

Description

There is no way to set value_initialized<T> object to the non-initialized value when T is a top-level const. I think there should be.

While it is not allowed to set a const value to a value after construction, one can usually set it to a value at construction time. The value_initialized<T> has no way to do that. Adding a constructor to it, which would allow the value to be set to something other than its default value initialized state, would solve this problem for T when it is a top-level const.

A practical use case is that value_initialized<T> might be used in a template class where T is a template parameter of that particular template class. If the end-user specifies that T is a top-level const, then an initial value for T passed to that template class's constructor can not be used to set a value_initialized<T> object to that value, making value_initialized<T> unusable in such a situation.

Adding a constructor to value_initialized<T> would solve that problem.

Attachments

value_init.patch Download (746 bytes) - added by Edward Diener <eld@…> 5 years ago.
Patch to latest SVN implementing fix
initialized.patch Download (4.5 KB) - added by niels_dekker 4 years ago.
Adds a new template class to value_init.hpp: boost::initialized<T>
initialized_documentation.patch Download (4.3 KB) - added by niels_dekker 4 years ago.
documentation for boost::initialized<T>
initialized_test.patch Download (6.2 KB) - added by niels_dekker 4 years ago.
tests for boost::initialized<T> (fixed a few typo's)

Change History

comment:1 Changed 5 years ago by niels_dekker

  • Cc niels_address_until_2010-10-10@… added
  • Owner changed from no-maintainer to fcacciola

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

Changed 5 years ago by Edward Diener <eld@…>

Patch to latest SVN implementing fix

comment:2 follow-up: ↓ 3 Changed 4 years ago by 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?

comment:3 in reply to: ↑ 2 Changed 4 years ago by Edward Diener <eld@…>

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.

comment:4 follow-up: ↓ 5 Changed 4 years ago by 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?

comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 4 years ago by Edward Diener <eld@…>

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.

comment:6 in reply to: ↑ 5 Changed 4 years ago by niels_dekker

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 :-)

comment:7 Changed 4 years ago by niels_dekker

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>.

Changed 4 years ago by niels_dekker

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

Changed 4 years ago by niels_dekker

documentation for boost::initialized<T>

Changed 4 years ago by niels_dekker

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

comment:8 Changed 4 years ago by niels_dekker

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

comment:9 Changed 4 years ago by niels_dekker

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

comment:10 Changed 4 years ago by niels_dekker

  • Status changed from new to closed
  • Resolution set to fixed

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

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.