Skip to content

Commit

Permalink
Set 3216 to Tentatively Ready
Browse files Browse the repository at this point in the history
  • Loading branch information
jwakely committed Oct 2, 2024
1 parent 6aa3d3f commit 0fbb143
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 33 deletions.
66 changes: 34 additions & 32 deletions xml/issue3210.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<!DOCTYPE issue SYSTEM "lwg-issue.dtd">

<issue num="3210" status="New">
<title><tt>allocate_shared</tt> is inconsistent about removing <tt>const</tt> from the pointer
<title><tt>allocate_shared</tt> is inconsistent about removing <tt>const</tt> from the pointer
passed to allocator <tt>construct</tt> and <tt>destroy</tt></title>
<section><sref ref="[util.smartptr.shared.create]"/></section>
<submitter>Billy O'Neal III</submitter>
Expand All @@ -11,34 +11,34 @@ passed to allocator <tt>construct</tt> and <tt>destroy</tt></title>

<discussion>
<p>
I implemented the fix for LWG <iref ref="3008"/> and Stephan pointed out there's an inconsistency here
I implemented the fix for LWG <iref ref="3008"/> and Stephan pointed out there's an inconsistency here
for <tt>allocate_shared&lt;const T&gt;</tt>.
<p/>
<sref ref="[util.smartptr.shared.create]"/> p3 says that the allocator construct call is done without removing
<i>cv</i> qualifiers, but <sref ref="[util.smartptr.shared.create]"/> p7.12 says that the <tt>destroy</tt>
<sref ref="[util.smartptr.shared.create]"/> p3 says that the allocator construct call is done without removing
<i>cv</i> qualifiers, but <sref ref="[util.smartptr.shared.create]"/> p7.12 says that the <tt>destroy</tt>
call is done with removed <i>cv</i> qualifiers.
<p/>
The fallback for <tt>allocator_traits::construct</tt> rejects <tt>const T*</tt> (since it <tt>static_casts</tt>
to <tt>void*</tt>), so the most likely outcome of attempting to do this today is to fail to compile, which
The fallback for <tt>allocator_traits::construct</tt> rejects <tt>const T*</tt> (since it <tt>static_casts</tt>
to <tt>void*</tt>), so the most likely outcome of attempting to do this today is to fail to compile, which
is a break with C++17.
<p/>
Our options are:
</p>
<ol>
<li><p>Fix the allocator model to deal with <tt>const</tt> elements somehow, which breaks compatibility
with existing allocators unprepared for <tt>const</tt> elements here. We would need to extend the allocator
<li><p>Fix the allocator model to deal with <tt>const</tt> elements somehow, which breaks compatibility
with existing allocators unprepared for <tt>const</tt> elements here. We would need to extend the allocator
requirements to allow <tt>const T*</tt> to be passed here, and fix our default to <tt>const_cast</tt>.</p></li>
<li><p>Fix <tt>allocate_shared</tt> to remove <tt>const</tt> before calling <tt>construct</tt>, which
changes the experience for C++17 customers because <tt>allocate_shared</tt> constructs a <tt>T</tt>
instead of a <tt>const T</tt>, but not in a way substantially different to edits
<li><p>Fix <tt>allocate_shared</tt> to remove <tt>const</tt> before calling <tt>construct</tt>, which
changes the experience for C++17 customers because <tt>allocate_shared</tt> constructs a <tt>T</tt>
instead of a <tt>const T</tt>, but not in a way substantially different to edits
<a href="https://wg21.link/p0674">P0674</a> already made here.</p></li>
<li><p>Back out <tt>allocate_shared</tt>'s interaction with this part of the allocator model (reverting
<li><p>Back out <tt>allocate_shared</tt>'s interaction with this part of the allocator model (reverting
this part of <a href="https://wg21.link/p0674">P0674</a> and reopening LWG <iref ref="3008"/>).</p></li>
<li><p>Go around the problem by prohibiting <tt>allocate_shared&lt;const T&gt;</tt>, which breaks
<li><p>Go around the problem by prohibiting <tt>allocate_shared&lt;const T&gt;</tt>, which breaks
existing C++17 customers.</p></li>
</ol>
<p>
Billy O'Neal argues that only (2) preserves the design intent <a href="https://wg21.link/p0674">P0674</a>
Billy O'Neal argues that only (2) preserves the design intent <a href="https://wg21.link/p0674">P0674</a>
while maintaining compatibility for most allocators and most C++17 customers.
<p/>
Peter Dimov argues that (1) isn't likely to break enough to matter.
Expand Down Expand Up @@ -80,18 +80,18 @@ template&lt;class T, class A, ...&gt;
<ol style="list-style-type: none">
<li><p>(7.1) &mdash; [&hellip;]</p></li>
<li><p>[&hellip;]</p></li>
<li><p>(7.5) &mdash; When a (sub)object of a non-array type <tt>U</tt> is specified to have an initial
value of <tt>v</tt>, or <tt>U(l...)</tt>, where <tt>l...</tt> is a list of constructor arguments,
<li><p>(7.5) &mdash; When a (sub)object of a non-array type <tt>U</tt> is specified to have an initial
value of <tt>v</tt>, or <tt>U(l...)</tt>, where <tt>l...</tt> is a list of constructor arguments,
<tt>allocate_shared</tt> shall initialize this (sub)object via the expression
</p>
<ol style="list-style-type: none">
<li><p>(7.5.1) &mdash; <tt>allocator_traits&lt;A2&gt;::construct(a2, pv, v)</tt> or</p></li>
<li><p>(7.5.2) &mdash; <tt>allocator_traits&lt;A2&gt;::construct(a2, pv, l...)</tt></p></li>
</ol>
<p>
respectively, where <tt>pv</tt> points to storage suitable to hold an object of type
<tt><ins>remove_cv_t&lt;</ins>U<ins>&gt;</ins></tt> and <tt>a2</tt> of type <tt>A2</tt> is a
rebound copy of the allocator <tt>a</tt> passed to <tt>allocate_shared</tt> such that its <tt>value_type</tt>
respectively, where <tt>pv</tt> points to storage suitable to hold an object of type
<tt><ins>remove_cv_t&lt;</ins>U<ins>&gt;</ins></tt> and <tt>a2</tt> of type <tt>A2</tt> is a
rebound copy of the allocator <tt>a</tt> passed to <tt>allocate_shared</tt> such that its <tt>value_type</tt>
is <tt>remove_cv_t&lt;U&gt;</tt>.
</p>
</li>
Expand All @@ -105,11 +105,13 @@ is <tt>remove_cv_t&lt;U&gt;</tt>.

<note>2024-04-13; Jiang An comments and provides improved wording</note>
<p>
The currently proposed resolution is meaningless, because "(allocated) storage suitable to hold an object of type
<tt>remove_cv_t&lt;U&gt;</tt>" is always "storage suitable to hold an object of type <tt>U</tt>", and vice versa.
Also, the current specification doesn't seem to specify the type of <tt>pv</tt> in the cases of <tt>allocator_shared</tt>,
The currently proposed resolution is meaningless, because "(allocated) storage suitable to hold an object of type
<tt>remove_cv_t&lt;U&gt;</tt>" is always "storage suitable to hold an object of type <tt>U</tt>", and vice versa.
Also, the current specification doesn't seem to specify the type of <tt>pv</tt> in the cases of <tt>allocator_shared</tt>,
because <tt>pv</tt> is merely specified to point some storage instead of an object.
</p>

<note>2024-10-02; will be resolved by issue <iref ref="3216"/>.</note>
</discussion>

<resolution>
Expand Down Expand Up @@ -148,27 +150,27 @@ template&amp;lt;class T, class A, ...>
<ol style="list-style-type: none">
<li><p>(7.1) &mdash; [&hellip;]</p></li>
<li><p>[&hellip;]</p></li>
<li><p>(7.5) &mdash; When a (sub)object of a non-array type <tt>U</tt> is specified to have an initial
value of <tt>v</tt>, or <tt>U(l...)</tt>, where <tt>l...</tt> is a list of constructor arguments,
<li><p>(7.5) &mdash; When a (sub)object of a non-array type <tt>U</tt> is specified to have an initial
value of <tt>v</tt>, or <tt>U(l...)</tt>, where <tt>l...</tt> is a list of constructor arguments,
<tt>allocate_shared</tt> shall initialize this (sub)object via the expression
</p>
<ol style="list-style-type: none">
<li><p>(7.5.1) &mdash; <tt>allocator_traits&lt;A2&gt;::construct(a2, pv, v)</tt> or</p></li>
<li><p>(7.5.2) &mdash; <tt>allocator_traits&lt;A2&gt;::construct(a2, pv, l...)</tt></p></li>
</ol>
<p>
respectively, where <tt>pv</tt> <ins>has type <tt>remove_cv_t&lt;U&gt;*</tt> and</ins> points to storage
suitable to hold an object of type <tt>U</tt> and <tt>a2</tt> of type <tt>A2</tt> is a rebound copy of
the allocator a passed to <tt>allocate_shared</tt> such that its <tt>value_type</tt> is
respectively, where <tt>pv</tt> <ins>has type <tt>remove_cv_t&lt;U&gt;*</tt> and</ins> points to storage
suitable to hold an object of type <tt>U</tt> and <tt>a2</tt> of type <tt>A2</tt> is a rebound copy of
the allocator a passed to <tt>allocate_shared</tt> such that its <tt>value_type</tt> is
<tt>remove_cv_t&lt;U&gt;</tt>.
</p>
</li>
<li><p>(7.6) &mdash; [&hellip;]</p></li>
<li><p>(7.7) &mdash; When a (sub)object of non-array type <tt>U</tt> is specified to have a default
initial value, <tt>allocate_shared</tt> shall initialize this (sub)object via the expression
<tt>allocator_traits&lt;A2&gt;::construct(a2, pv)</tt>, where <tt>pv</tt> <ins>has type <tt>remove_cv_t&lt;U&gt;*</tt>
and</ins> points to storage suitable to hold an object of type <tt>U</tt> and <tt>a2</tt> of type <tt>A2</tt>
is a rebound copy of the allocator a passed to <tt>allocate_shared</tt> such that its <tt>value_type</tt>
<li><p>(7.7) &mdash; When a (sub)object of non-array type <tt>U</tt> is specified to have a default
initial value, <tt>allocate_shared</tt> shall initialize this (sub)object via the expression
<tt>allocator_traits&lt;A2&gt;::construct(a2, pv)</tt>, where <tt>pv</tt> <ins>has type <tt>remove_cv_t&lt;U&gt;*</tt>
and</ins> points to storage suitable to hold an object of type <tt>U</tt> and <tt>a2</tt> of type <tt>A2</tt>
is a rebound copy of the allocator a passed to <tt>allocate_shared</tt> such that its <tt>value_type</tt>
is <tt>remove_cv_t&lt;U&gt;</tt>.
</p>
</li>
Expand Down
8 changes: 7 additions & 1 deletion xml/issue3216.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version='1.0' encoding='utf-8' standalone='no'?>
<!DOCTYPE issue SYSTEM "lwg-issue.dtd">

<issue num="3216" status="New">
<issue num="3216" status="Tentatively Ready">
<title>Rebinding the allocator before calling <tt>construct</tt>/<tt>destroy</tt> in <tt>allocate_shared</tt></title>
<section><sref ref="[util.smartptr.shared.create]"/></section>
<submitter>Billy O'Neal III</submitter>
Expand Down Expand Up @@ -110,6 +110,12 @@ and `pu` is <code>remove_cv_t&lt;U&gt;*</code>.
Accepting this proposed resolution would also resolve issue <iref ref="3210"/>.
</p>


<note>2024-10-02; Reflector poll</note>
<p>
Set status to Tentatively Ready after six votes in favour during reflector poll.
</p>

</discussion>

<resolution>
Expand Down

0 comments on commit 0fbb143

Please sign in to comment.