Re: [Patch] Inline Variables for the Standard Library (p0607r0)

[Jonathan Wakely]

On 23/03/17 13:47 -0400, Tim Song wrote:

On Thu, Mar 23, 2017 at 1:45 PM, Jonathan Wakely  wrote:

+ // NOTE: This makes use of the fact that we know how moveable
+ // is implemented on pair, and also vector. If the implementation
+ // changes this test may begin to fail.


Maybe drop this comment?


Ha, yes, I'll do so. Thanks.

Re: [Patch] Inline Variables for the Standard Library (p0607r0)

[Tim Song]

On Thu, Mar 23, 2017 at 1:45 PM, Jonathan Wakely  wrote:
> + // NOTE: This makes use of the fact that we know how moveable
> + // is implemented on pair, and also vector. If the implementation
> + // changes this test may begin to fail.

Maybe drop this comment?

Re: [Patch] Inline Variables for the Standard Library (p0607r0)

[Jonathan Wakely]

On 12/03/17 01:04 +0100, Daniel Krügler wrote:

2017-03-11 23:14 GMT+01:00 Daniel Krügler :

2017-03-11 23:09 GMT+01:00 Tim Song :

On Sat, Mar 11, 2017 at 3:37 PM, Daniel Krügler
 wrote:

2017-03-11 21:23 GMT+01:00 Tim Song :

On Sat, Mar 11, 2017 at 1:32 PM, Daniel Krügler
 wrote:

This patch applies inline to all namespace scope const variables
according to options A and B2 voted in during the Kona meeting, see:

http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html

During that work it has been found that std::ignore was not declared
constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773),
which was fixed as well.


Just adding constexpr to std::ignore does ~nothing. The assignment
operator needs to be made constexpr too; there is really no other
reason to use std::ignore.


There is nothing in the resolution of the issue (Nor in the discussion
that argues for the change) that says so. Yes, I considered to make
the assignment function template constexpr, but decided against it,
because the wording of the issue doesn't have this requirement). I'm
also aware of a test-case which would show the difference, but again:
This was not what the issue said. In fact I'm in the process to open a
new library issue that should impose that additional requirement.

- Daniel


Well, technically speaking, the issue resolution doesn't even
guarantee that the use case mentioned in the issue would compile since
the standard says nothing about whether the copy constructor of
decltype(std::ignore) is constexpr, or even whether it can be copied
at all. Yes, std::ignore is underspecified, but I'm not sure I see the
point in waiting; it's a completely conforming change and IMHO the
issue's intent is clearly to make std::ignore fully usable in
constexpr functions.

Also, the only specification we have for std::ignore's semantics is
"when an argument in t is ignore, assigning any value to the
corresponding tuple element has no effect". I think one can argue that
"causes an expression to be non-constant" is an "effect".

Re "new library issue", we already have issue 2933.


Good point, it already exists ;-)


Revised patch attached.


The patch had a couple of typos, "onstexpr" in one place, and using
_GLIBCXX17_CONSTEXPR instead of _GLIBCXX17_INLINE in another file.

Here's what I've tested and am going to commit.


commit e17662621cde44bc6d367fafdba4b4f05f74df05
Author: Jonathan Wakely 
Date:   Thu Mar 23 11:22:48 2017 +

Implement P0607R0 "Inline Variables for Standard Library" for C++17

2017-03-23  Daniel Kruegler  

	* include/bits/c++config (_GLIBCXX17_INLINE): Define.
	* include/bits/regex_constants.h (All std::regex_constants constants):
	Add _GLIBCXX17_INLINE as per P0607R0.
	* include/bits/std_mutex.h (defer_lock, try_to_lock, adopt_lock):
	Likewise.
	* include/bits/stl_pair.h (piecewise_construct): Likewise.
	* include/bits/uses_allocator.h (allocator_arg, uses_allocator_v)
	(__is_uses_allocator_constructible_v)
	(__is_nothrow_uses_allocator_constructible_v): Likewise.
	* include/std/chrono (treat_as_floating_point_v): Likewise.
	* include/std/functional (is_bind_expression_v, is_placeholder_v):
	Likewise.
	* include/std/optional (nullopt): Likewise.
	* include/std/ratio (ratio_equal_v, ratio_not_equal_v, ratio_less_v)
	ratio_less_equal_v, ratio_greater_v, ratio_greater_equal_v): Likewise.
	* include/std/system_error (is_error_code_enum_v)
	(is_error_condition_enum_v): Likewise.
	* include/std/tuple (tuple_size_v, ignore): Likewise.
	(ignore): Declare ignore constexpr as per LWG 2773, declare assignment
	constexpr as per LWG 2933.
	* include/std/type_traits (All variable templates): Add
	_GLIBCXX17_INLINE as per P0607R0.
	* include/std/variant (variant_size_v, variant_npos, __index_of_v)
	(__tuple_count_v, __exactly_once): Likewise.
	* testsuite/18_support/headers/new/synopsis.cc
	(hardware_destructive_interference_size)
	(hardware_constructive_interference_size): Likewise for commented-out
	variables.
	* testsuite/20_util/tuple/creation_functions/constexpr.cc: Add new
	test function for constexpr std::ignore (LWG 2773).
	* testsuite/20_util/tuple/creation_functions/constexpr_cpp14.cc: New
	test for LWG 2933.

diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 3b694e0..8ca6b03 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -122,6 +122,14 @@
 # endif
 #endif
 
+#ifndef _GLIBCXX17_INLINE
+# if __cplusplus > 201402L
+#  define _GLIBCXX17_INLINE inline
+# else
+#  define _GLIBCXX17_INLINE
+# endif
+#endif
+
 // Macro for noexcept, to support in mixed 03/0x mode.
 #ifndef _GLIBCXX_NOEXCEPT
 # if 

Re: [Patch] Inline Variables for the Standard Library (p0607r0)

[Daniel Krügler]

2017-03-12 1:04 GMT+01:00 Daniel Krügler :
> 2017-03-11 23:14 GMT+01:00 Daniel Krügler :
>> 2017-03-11 23:09 GMT+01:00 Tim Song :
>>> On Sat, Mar 11, 2017 at 3:37 PM, Daniel Krügler
>>>  wrote:
 2017-03-11 21:23 GMT+01:00 Tim Song :
> On Sat, Mar 11, 2017 at 1:32 PM, Daniel Krügler
>  wrote:
>> This patch applies inline to all namespace scope const variables
>> according to options A and B2 voted in during the Kona meeting, see:
>>
>> http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html
>>
>> During that work it has been found that std::ignore was not declared
>> constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773),
>> which was fixed as well.
>
> Just adding constexpr to std::ignore does ~nothing. The assignment
> operator needs to be made constexpr too; there is really no other
> reason to use std::ignore.

 There is nothing in the resolution of the issue (Nor in the discussion
 that argues for the change) that says so. Yes, I considered to make
 the assignment function template constexpr, but decided against it,
 because the wording of the issue doesn't have this requirement). I'm
 also aware of a test-case which would show the difference, but again:
 This was not what the issue said. In fact I'm in the process to open a
 new library issue that should impose that additional requirement.

 - Daniel
>>>
>>> Well, technically speaking, the issue resolution doesn't even
>>> guarantee that the use case mentioned in the issue would compile since
>>> the standard says nothing about whether the copy constructor of
>>> decltype(std::ignore) is constexpr, or even whether it can be copied
>>> at all. Yes, std::ignore is underspecified, but I'm not sure I see the
>>> point in waiting; it's a completely conforming change and IMHO the
>>> issue's intent is clearly to make std::ignore fully usable in
>>> constexpr functions.
>>>
>>> Also, the only specification we have for std::ignore's semantics is
>>> "when an argument in t is ignore, assigning any value to the
>>> corresponding tuple element has no effect". I think one can argue that
>>> "causes an expression to be non-constant" is an "effect".
>>>
>>> Re "new library issue", we already have issue 2933.
>>
>> Good point, it already exists ;-)
>
> Revised patch attached.

I would just like to point out that I'm on vacation from Friday on for
two weeks, so if any changes to this patch are requested, I will work
on them after my return.

Thanks,

- Daniel

Re: [Patch] Inline Variables for the Standard Library (p0607r0)

[Daniel Krügler]

2017-03-11 23:14 GMT+01:00 Daniel Krügler :
> 2017-03-11 23:09 GMT+01:00 Tim Song :
>> On Sat, Mar 11, 2017 at 3:37 PM, Daniel Krügler
>>  wrote:
>>> 2017-03-11 21:23 GMT+01:00 Tim Song :
 On Sat, Mar 11, 2017 at 1:32 PM, Daniel Krügler
  wrote:
> This patch applies inline to all namespace scope const variables
> according to options A and B2 voted in during the Kona meeting, see:
>
> http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html
>
> During that work it has been found that std::ignore was not declared
> constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773),
> which was fixed as well.

 Just adding constexpr to std::ignore does ~nothing. The assignment
 operator needs to be made constexpr too; there is really no other
 reason to use std::ignore.
>>>
>>> There is nothing in the resolution of the issue (Nor in the discussion
>>> that argues for the change) that says so. Yes, I considered to make
>>> the assignment function template constexpr, but decided against it,
>>> because the wording of the issue doesn't have this requirement). I'm
>>> also aware of a test-case which would show the difference, but again:
>>> This was not what the issue said. In fact I'm in the process to open a
>>> new library issue that should impose that additional requirement.
>>>
>>> - Daniel
>>
>> Well, technically speaking, the issue resolution doesn't even
>> guarantee that the use case mentioned in the issue would compile since
>> the standard says nothing about whether the copy constructor of
>> decltype(std::ignore) is constexpr, or even whether it can be copied
>> at all. Yes, std::ignore is underspecified, but I'm not sure I see the
>> point in waiting; it's a completely conforming change and IMHO the
>> issue's intent is clearly to make std::ignore fully usable in
>> constexpr functions.
>>
>> Also, the only specification we have for std::ignore's semantics is
>> "when an argument in t is ignore, assigning any value to the
>> corresponding tuple element has no effect". I think one can argue that
>> "causes an expression to be non-constant" is an "effect".
>>
>> Re "new library issue", we already have issue 2933.
>
> Good point, it already exists ;-)

Revised patch attached.

> - Daniel


ChangeLog_p0607r0.patch
Description: Binary data


p0607r0.patch
Description: Binary data

Re: [Patch] Inline Variables for the Standard Library (p0607r0)

[Daniel Krügler]

2017-03-11 23:09 GMT+01:00 Tim Song :
> On Sat, Mar 11, 2017 at 3:37 PM, Daniel Krügler
>  wrote:
>> 2017-03-11 21:23 GMT+01:00 Tim Song :
>>> On Sat, Mar 11, 2017 at 1:32 PM, Daniel Krügler
>>>  wrote:
 This patch applies inline to all namespace scope const variables
 according to options A and B2 voted in during the Kona meeting, see:

 http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html

 During that work it has been found that std::ignore was not declared
 constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773),
 which was fixed as well.
>>>
>>> Just adding constexpr to std::ignore does ~nothing. The assignment
>>> operator needs to be made constexpr too; there is really no other
>>> reason to use std::ignore.
>>
>> There is nothing in the resolution of the issue (Nor in the discussion
>> that argues for the change) that says so. Yes, I considered to make
>> the assignment function template constexpr, but decided against it,
>> because the wording of the issue doesn't have this requirement). I'm
>> also aware of a test-case which would show the difference, but again:
>> This was not what the issue said. In fact I'm in the process to open a
>> new library issue that should impose that additional requirement.
>>
>> - Daniel
>
> Well, technically speaking, the issue resolution doesn't even
> guarantee that the use case mentioned in the issue would compile since
> the standard says nothing about whether the copy constructor of
> decltype(std::ignore) is constexpr, or even whether it can be copied
> at all. Yes, std::ignore is underspecified, but I'm not sure I see the
> point in waiting; it's a completely conforming change and IMHO the
> issue's intent is clearly to make std::ignore fully usable in
> constexpr functions.
>
> Also, the only specification we have for std::ignore's semantics is
> "when an argument in t is ignore, assigning any value to the
> corresponding tuple element has no effect". I think one can argue that
> "causes an expression to be non-constant" is an "effect".
>
> Re "new library issue", we already have issue 2933.

Good point, it already exists ;-)

- Daniel

Re: [Patch] Inline Variables for the Standard Library (p0607r0)

[Tim Song]

On Sat, Mar 11, 2017 at 3:37 PM, Daniel Krügler
 wrote:
> 2017-03-11 21:23 GMT+01:00 Tim Song :
>> On Sat, Mar 11, 2017 at 1:32 PM, Daniel Krügler
>>  wrote:
>>> This patch applies inline to all namespace scope const variables
>>> according to options A and B2 voted in during the Kona meeting, see:
>>>
>>> http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html
>>>
>>> During that work it has been found that std::ignore was not declared
>>> constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773),
>>> which was fixed as well.
>>
>> Just adding constexpr to std::ignore does ~nothing. The assignment
>> operator needs to be made constexpr too; there is really no other
>> reason to use std::ignore.
>
> There is nothing in the resolution of the issue (Nor in the discussion
> that argues for the change) that says so. Yes, I considered to make
> the assignment function template constexpr, but decided against it,
> because the wording of the issue doesn't have this requirement). I'm
> also aware of a test-case which would show the difference, but again:
> This was not what the issue said. In fact I'm in the process to open a
> new library issue that should impose that additional requirement.
>
> - Daniel

Well, technically speaking, the issue resolution doesn't even
guarantee that the use case mentioned in the issue would compile since
the standard says nothing about whether the copy constructor of
decltype(std::ignore) is constexpr, or even whether it can be copied
at all. Yes, std::ignore is underspecified, but I'm not sure I see the
point in waiting; it's a completely conforming change and IMHO the
issue's intent is clearly to make std::ignore fully usable in
constexpr functions.

Also, the only specification we have for std::ignore's semantics is
"when an argument in t is ignore, assigning any value to the
corresponding tuple element has no effect". I think one can argue that
"causes an expression to be non-constant" is an "effect".

Re "new library issue", we already have issue 2933.

Tim

Re: [Patch] Inline Variables for the Standard Library (p0607r0)

[Daniel Krügler]

2017-03-11 21:23 GMT+01:00 Tim Song :
> On Sat, Mar 11, 2017 at 1:32 PM, Daniel Krügler
>  wrote:
>> This patch applies inline to all namespace scope const variables
>> according to options A and B2 voted in during the Kona meeting, see:
>>
>> http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html
>>
>> During that work it has been found that std::ignore was not declared
>> constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773),
>> which was fixed as well.
>
> Just adding constexpr to std::ignore does ~nothing. The assignment
> operator needs to be made constexpr too; there is really no other
> reason to use std::ignore.

There is nothing in the resolution of the issue (Nor in the discussion
that argues for the change) that says so. Yes, I considered to make
the assignment function template constexpr, but decided against it,
because the wording of the issue doesn't have this requirement). I'm
also aware of a test-case which would show the difference, but again:
This was not what the issue said. In fact I'm in the process to open a
new library issue that should impose that additional requirement.

- Daniel

Re: [Patch] Inline Variables for the Standard Library (p0607r0)

[Tim Song]

On Sat, Mar 11, 2017 at 1:32 PM, Daniel Krügler
 wrote:
> This patch applies inline to all namespace scope const variables
> according to options A and B2 voted in during the Kona meeting, see:
>
> http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html
>
> During that work it has been found that std::ignore was not declared
> constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773),
> which was fixed as well.

Just adding constexpr to std::ignore does ~nothing. The assignment
operator needs to be made constexpr too; there is really no other
reason to use std::ignore.

[Patch] Inline Variables for the Standard Library (p0607r0)

[Daniel Krügler]

This patch applies inline to all namespace scope const variables
according to options A and B2 voted in during the Kona meeting, see:

http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html

During that work it has been found that std::ignore was not declared
constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773),
which was fixed as well.

The patch suggests to apply the same fix to implementation-private
constants such as __index_of_v, for example, as well.

Due to the large number of affected variables in header
include/bits/regex_constants.h and include/type_traits, I used a
shortened description of the changes in the ChangeLog instead of
enumerating each constant individually, I hope that this is ok.

Please note that I added the newly introduced _GLIBCXX17_INLINE also
to the commented references of hardware_destructive_interference_size
and hardware_constructive_interference_size in
testsuite/18_support/headers/new/synopsis.cc as a reminder when these
constants will be introduced in the future.

This patch is *untested*, because I cannot make the tests run on my
Windows system.

- Daniel


ChangeLog_p0607r0.patch
Description: Binary data


p0607r0.patch
Description: Binary data