This is an archive of the discontinued LLVM Phabricator instance.

Differential D114243

[libc++] Fix feature test macro for __cpp_lib_to_chars
ClosedPublic

Authored by ldionne on Nov 19 2021, 6:55 AM.

Details

Reviewers
• Quuxplusone
Mordante
Group Reviewers
Restricted Project
Commits
rGe0f58444e126: [libc++] Fix feature test macro for __cpp_lib_to_chars
Summary

We would have been defining it in <utility> instead of <charconv>. For
the time being, this doesn't change anything since we don't implement
the feature test macro anyways.

Also, as a fly-by, this removes obsolete feature test macro tests. There
was a brief time back in the days when we wrote feature test macro tests
manually. In particular, we had test files for cpp_lib_to_chars and
cpp_lib_memory_resource. Since we now have a principled way of generating
these tests with scripts, this commit removes the obsolete (and empty)
tests for these two feature test macros.

Diff Detail

Event Timeline

ldionne requested review of this revision.Nov 19 2021, 6:55 AM
ldionne created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2021, 6:55 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
• Quuxplusone requested changes to this revision.Nov 19 2021, 7:54 AM
• Quuxplusone added a subscriber: • Quuxplusone.

This smelled fishy to me so I investigated a tiny bit. I believe we're defining __cpp_lib_to_chars in the wrong header (utility should be charconv). I didn't dig deeper than that, but I'd encourage you to. :)

This revision now requires changes to proceed.Nov 19 2021, 7:54 AM
ldionne added a comment.Nov 19 2021, 8:00 AM
In D114243#3142866, @Quuxplusone wrote:

This smelled fishy to me so I investigated a tiny bit. I believe we're defining __cpp_lib_to_chars in the wrong header (utility should be charconv). I didn't dig deeper than that, but I'd encourage you to. :)

You seem to be right (http://eel.is/c++draft/version.syn#lib:__cpp_lib_to_chars). Let me correct that.

But nonetheless, these manually written tests should go, do you agree?

ldionne updated this revision to Diff 388507.Nov 19 2021, 8:00 AM
ldionne retitled this revision from [libc++] Remove obsolete feature test macro tests to [libc++] Fix feature test macro for __cpp_lib_to_chars.
ldionne edited the summary of this revision. (Show Details)

Address Arthur's comments.

Herald added a subscriber: arichardson. · View Herald TranscriptNov 19 2021, 8:00 AM
• Quuxplusone accepted this revision as: • Quuxplusone.Nov 19 2021, 8:15 AM

should go, do you agree?

Definitely, yes. You might do one more pass through wherever the feature-test macros are standardized, just to see if there's anything else that should be defined in <charconv> that we've missed.

Mordante accepted this revision.Nov 19 2021, 9:29 AM
Mordante added a subscriber: Mordante.

Nice catch. LGTM, I would be slightly concerned about removing it from the <utility> header if the feature were completely implemented.

This revision is now accepted and ready to land.Nov 19 2021, 9:29 AM
Harbormaster completed remote builds in B135125: Diff 388507.Nov 19 2021, 9:54 AM
ldionne added a comment.Nov 19 2021, 11:26 AM
In D114243#3142923, @Quuxplusone wrote:

should go, do you agree?

Definitely, yes. You might do one more pass through wherever the feature-test macros are standardized, just to see if there's anything else that should be defined in <charconv> that we've missed.

That's the only FTM in <charconv> AFAICT: http://eel.is/c++draft/version.syn

In D114243#3143089, @Mordante wrote:

Nice catch. LGTM, I would be slightly concerned about removing it from the <utility> header if the feature were completely implemented.

Yeah, agreed, luckily it has never been defined.

Thank you both!

Closed by commit rGe0f58444e126: [libc++] Fix feature test macro for __cpp_lib_to_chars (authored by ldionne). · Explain WhyNov 19 2021, 11:26 AM
This revision was automatically updated to reflect the committed changes.
ldionne added a commit: rGe0f58444e126: [libc++] Fix feature test macro for __cpp_lib_to_chars.
Mordante added a comment.Nov 19 2021, 12:19 PM
In D114243#3143509, @ldionne wrote:
In D114243#3143089, @Mordante wrote:

Nice catch. LGTM, I would be slightly concerned about removing it from the <utility> header if the feature were completely implemented.

Yeah, agreed, luckily it has never been defined.

Well I have to disagree with that assessment, I would have loved us to have completed C++17 support by now.
(/me will try to complete to_chars float real soon now.)

Revision Contents

PathSize
libcxx/
include/
2 lines
test/
std/
language.support/
support.limits/
support.limits.general/
84 lines
48 lines
utils/
2 lines

Diff 388507

libcxx/include/version

Show First 20 Lines • Show All 143 Lines • ▼ Show 20 Lines
__cpp_lib_string_contains 202011L <string> <string_view> __cpp_lib_string_contains 202011L <string> <string_view>
__cpp_lib_string_udls 201304L <string> __cpp_lib_string_udls 201304L <string>
__cpp_lib_string_view 201803L <string> <string_view> __cpp_lib_string_view 201803L <string> <string_view>
201606L // C++17 201606L // C++17
__cpp_lib_syncbuf 201803L <syncstream> __cpp_lib_syncbuf 201803L <syncstream>
__cpp_lib_three_way_comparison 201907L <compare> __cpp_lib_three_way_comparison 201907L <compare>
__cpp_lib_to_address 201711L <memory> __cpp_lib_to_address 201711L <memory>
__cpp_lib_to_array 201907L <array> __cpp_lib_to_array 201907L <array>
__cpp_lib_to_chars 201611L <utility> __cpp_lib_to_chars 201611L <charconv>
__cpp_lib_to_underlying 202102L <utility> __cpp_lib_to_underlying 202102L <utility>
__cpp_lib_transformation_trait_aliases 201304L <type_traits> __cpp_lib_transformation_trait_aliases 201304L <type_traits>
__cpp_lib_transparent_operators 201510L <functional> <memory> __cpp_lib_transparent_operators 201510L <functional> <memory>
201210L // C++14 201210L // C++14
__cpp_lib_tuple_element_t 201402L <tuple> __cpp_lib_tuple_element_t 201402L <tuple>
__cpp_lib_tuples_by_type 201304L <tuple> <utility> __cpp_lib_tuples_by_type 201304L <tuple> <utility>
__cpp_lib_type_trait_variable_templates 201510L <type_traits> __cpp_lib_type_trait_variable_templates 201510L <type_traits>
__cpp_lib_uncaught_exceptions 201411L <exception> __cpp_lib_uncaught_exceptions 201411L <exception>
▲ Show 20 Lines • Show All 196 Lines • Show Last 20 Lines

libcxx/test/std/language.support/support.limits/support.limits.general/charconv.pass.cpp

  • This file was deleted.
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// <charconv> feature macros
/* Constant Value
__cpp_lib_to_chars 201611L
*/
#include <charconv>
#include <cassert>
#include "test_macros.h"
int main(int, char**)
{
// ensure that the macros that are supposed to be defined in <utility> are defined.
/*
#if !defined(__cpp_lib_fooby)
# error "__cpp_lib_fooby is not defined"
#elif __cpp_lib_fooby < 201606L
# error "__cpp_lib_fooby has an invalid value"
#endif
*/
return 0;
}

libcxx/test/std/language.support/support.limits/support.limits.general/charconv.version.pass.cpp

  • This file was added.
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// WARNING: This test was generated by generate_feature_test_macro_components.py
// and should not be edited manually.
//
// clang-format off
// <charconv>
// Test the feature test macros defined by <charconv>
/* Constant Value
__cpp_lib_to_chars 201611L [C++17]
*/
#include <charconv>
#include "test_macros.h"
#if TEST_STD_VER < 14
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined before c++17"
# endif
#elif TEST_STD_VER == 14
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined before c++17"
# endif
#elif TEST_STD_VER == 17
# if !defined(_LIBCPP_VERSION)
# ifndef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should be defined in c++17"
# endif
# if __cpp_lib_to_chars != 201611L
# error "__cpp_lib_to_chars should have the value 201611L in c++17"
# endif
# else // _LIBCPP_VERSION
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined because it is unimplemented in libc++!"
# endif
# endif
#elif TEST_STD_VER == 20
# if !defined(_LIBCPP_VERSION)
# ifndef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should be defined in c++20"
# endif
# if __cpp_lib_to_chars != 201611L
# error "__cpp_lib_to_chars should have the value 201611L in c++20"
# endif
# else // _LIBCPP_VERSION
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined because it is unimplemented in libc++!"
# endif
# endif
#elif TEST_STD_VER > 20
# if !defined(_LIBCPP_VERSION)
# ifndef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should be defined in c++2b"
# endif
# if __cpp_lib_to_chars != 201611L
# error "__cpp_lib_to_chars should have the value 201611L in c++2b"
# endif
# else // _LIBCPP_VERSION
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined because it is unimplemented in libc++!"
# endif
# endif
#endif // TEST_STD_VER > 20
int main(int, char**) { return 0; }

libcxx/test/std/language.support/support.limits/support.limits.general/memory_resource.version.pass.cpp

  • This file was deleted.
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// <memory_resource> feature macros
/* Constant Value
__cpp_lib_memory_resource 201603L
*/
// XFAIL
// #include <memory_resource>
#include <cassert>
#include "test_macros.h"
int main(int, char**)
{
// ensure that the macros that are supposed to be defined in <memory_resource> are defined.
/*
#if !defined(__cpp_lib_fooby)
# error "__cpp_lib_fooby is not defined"
#elif __cpp_lib_fooby < 201606L
# error "__cpp_lib_fooby has an invalid value"
#endif
*/
return 0;
}

libcxx/test/std/language.support/support.limits/support.limits.general/utility.version.pass.cpp

Show All 15 Lines
// Test the feature test macros defined by <utility> // Test the feature test macros defined by <utility>
/* Constant Value /* Constant Value
__cpp_lib_as_const 201510L [C++17] __cpp_lib_as_const 201510L [C++17]
__cpp_lib_constexpr_utility 201811L [C++20] __cpp_lib_constexpr_utility 201811L [C++20]
__cpp_lib_exchange_function 201304L [C++14] __cpp_lib_exchange_function 201304L [C++14]
__cpp_lib_integer_comparison_functions 202002L [C++20] __cpp_lib_integer_comparison_functions 202002L [C++20]
__cpp_lib_integer_sequence 201304L [C++14] __cpp_lib_integer_sequence 201304L [C++14]
__cpp_lib_to_chars 201611L [C++17]
__cpp_lib_to_underlying 202102L [C++2b] __cpp_lib_to_underlying 202102L [C++2b]
__cpp_lib_tuples_by_type 201304L [C++14] __cpp_lib_tuples_by_type 201304L [C++14]
*/ */
#include <utility> #include <utility>
#include "test_macros.h" #include "test_macros.h"
#if TEST_STD_VER < 14 #if TEST_STD_VER < 14
Show All 13 Lines
# ifdef __cpp_lib_integer_comparison_functions # ifdef __cpp_lib_integer_comparison_functions
# error "__cpp_lib_integer_comparison_functions should not be defined before c++20" # error "__cpp_lib_integer_comparison_functions should not be defined before c++20"
# endif # endif
# ifdef __cpp_lib_integer_sequence # ifdef __cpp_lib_integer_sequence
# error "__cpp_lib_integer_sequence should not be defined before c++14" # error "__cpp_lib_integer_sequence should not be defined before c++14"
# endif # endif
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined before c++17"
# endif
# ifdef __cpp_lib_to_underlying # ifdef __cpp_lib_to_underlying
# error "__cpp_lib_to_underlying should not be defined before c++2b" # error "__cpp_lib_to_underlying should not be defined before c++2b"
# endif # endif
# ifdef __cpp_lib_tuples_by_type # ifdef __cpp_lib_tuples_by_type
# error "__cpp_lib_tuples_by_type should not be defined before c++14" # error "__cpp_lib_tuples_by_type should not be defined before c++14"
# endif # endif
Show All 20 Lines
# ifndef __cpp_lib_integer_sequence # ifndef __cpp_lib_integer_sequence
# error "__cpp_lib_integer_sequence should be defined in c++14" # error "__cpp_lib_integer_sequence should be defined in c++14"
# endif # endif
# if __cpp_lib_integer_sequence != 201304L # if __cpp_lib_integer_sequence != 201304L
# error "__cpp_lib_integer_sequence should have the value 201304L in c++14" # error "__cpp_lib_integer_sequence should have the value 201304L in c++14"
# endif # endif
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined before c++17"
# endif
# ifdef __cpp_lib_to_underlying # ifdef __cpp_lib_to_underlying
# error "__cpp_lib_to_underlying should not be defined before c++2b" # error "__cpp_lib_to_underlying should not be defined before c++2b"
# endif # endif
# ifndef __cpp_lib_tuples_by_type # ifndef __cpp_lib_tuples_by_type
# error "__cpp_lib_tuples_by_type should be defined in c++14" # error "__cpp_lib_tuples_by_type should be defined in c++14"
# endif # endif
# if __cpp_lib_tuples_by_type != 201304L # if __cpp_lib_tuples_by_type != 201304L
Show All 26 Lines
# ifndef __cpp_lib_integer_sequence # ifndef __cpp_lib_integer_sequence
# error "__cpp_lib_integer_sequence should be defined in c++17" # error "__cpp_lib_integer_sequence should be defined in c++17"
# endif # endif
# if __cpp_lib_integer_sequence != 201304L # if __cpp_lib_integer_sequence != 201304L
# error "__cpp_lib_integer_sequence should have the value 201304L in c++17" # error "__cpp_lib_integer_sequence should have the value 201304L in c++17"
# endif # endif
# if !defined(_LIBCPP_VERSION)
# ifndef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should be defined in c++17"
# endif
# if __cpp_lib_to_chars != 201611L
# error "__cpp_lib_to_chars should have the value 201611L in c++17"
# endif
# else // _LIBCPP_VERSION
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined because it is unimplemented in libc++!"
# endif
# endif
# ifdef __cpp_lib_to_underlying # ifdef __cpp_lib_to_underlying
# error "__cpp_lib_to_underlying should not be defined before c++2b" # error "__cpp_lib_to_underlying should not be defined before c++2b"
# endif # endif
# ifndef __cpp_lib_tuples_by_type # ifndef __cpp_lib_tuples_by_type
# error "__cpp_lib_tuples_by_type should be defined in c++17" # error "__cpp_lib_tuples_by_type should be defined in c++17"
# endif # endif
# if __cpp_lib_tuples_by_type != 201304L # if __cpp_lib_tuples_by_type != 201304L
Show All 38 Lines
# ifndef __cpp_lib_integer_sequence # ifndef __cpp_lib_integer_sequence
# error "__cpp_lib_integer_sequence should be defined in c++20" # error "__cpp_lib_integer_sequence should be defined in c++20"
# endif # endif
# if __cpp_lib_integer_sequence != 201304L # if __cpp_lib_integer_sequence != 201304L
# error "__cpp_lib_integer_sequence should have the value 201304L in c++20" # error "__cpp_lib_integer_sequence should have the value 201304L in c++20"
# endif # endif
# if !defined(_LIBCPP_VERSION)
# ifndef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should be defined in c++20"
# endif
# if __cpp_lib_to_chars != 201611L
# error "__cpp_lib_to_chars should have the value 201611L in c++20"
# endif
# else // _LIBCPP_VERSION
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined because it is unimplemented in libc++!"
# endif
# endif
# ifdef __cpp_lib_to_underlying # ifdef __cpp_lib_to_underlying
# error "__cpp_lib_to_underlying should not be defined before c++2b" # error "__cpp_lib_to_underlying should not be defined before c++2b"
# endif # endif
# ifndef __cpp_lib_tuples_by_type # ifndef __cpp_lib_tuples_by_type
# error "__cpp_lib_tuples_by_type should be defined in c++20" # error "__cpp_lib_tuples_by_type should be defined in c++20"
# endif # endif
# if __cpp_lib_tuples_by_type != 201304L # if __cpp_lib_tuples_by_type != 201304L
Show All 38 Lines
# ifndef __cpp_lib_integer_sequence # ifndef __cpp_lib_integer_sequence
# error "__cpp_lib_integer_sequence should be defined in c++2b" # error "__cpp_lib_integer_sequence should be defined in c++2b"
# endif # endif
# if __cpp_lib_integer_sequence != 201304L # if __cpp_lib_integer_sequence != 201304L
# error "__cpp_lib_integer_sequence should have the value 201304L in c++2b" # error "__cpp_lib_integer_sequence should have the value 201304L in c++2b"
# endif # endif
# if !defined(_LIBCPP_VERSION)
# ifndef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should be defined in c++2b"
# endif
# if __cpp_lib_to_chars != 201611L
# error "__cpp_lib_to_chars should have the value 201611L in c++2b"
# endif
# else // _LIBCPP_VERSION
# ifdef __cpp_lib_to_chars
# error "__cpp_lib_to_chars should not be defined because it is unimplemented in libc++!"
# endif
# endif
# ifndef __cpp_lib_to_underlying # ifndef __cpp_lib_to_underlying
# error "__cpp_lib_to_underlying should be defined in c++2b" # error "__cpp_lib_to_underlying should be defined in c++2b"
# endif # endif
# if __cpp_lib_to_underlying != 202102L # if __cpp_lib_to_underlying != 202102L
# error "__cpp_lib_to_underlying should have the value 202102L in c++2b" # error "__cpp_lib_to_underlying should have the value 202102L in c++2b"
# endif # endif
# ifndef __cpp_lib_tuples_by_type # ifndef __cpp_lib_tuples_by_type
Show All 9 Lines

libcxx/utils/generate_feature_test_macro_components.py

Show First 20 Lines • Show All 590 Lines • ▼ Show 20 Linesfeature_test_macros = [ add_version_header(x) for x in [
"headers": ["memory"], "headers": ["memory"],
}, { }, {
"name": "__cpp_lib_to_array", "name": "__cpp_lib_to_array",
"values": { "c++20": 201907 }, "values": { "c++20": 201907 },
"headers": ["array"], "headers": ["array"],
}, { }, {
"name": "__cpp_lib_to_chars", "name": "__cpp_lib_to_chars",
"values": { "c++17": 201611 }, "values": { "c++17": 201611 },
"headers": ["utility"], "headers": ["charconv"],
"unimplemented": True, "unimplemented": True,
}, { }, {
"name": "__cpp_lib_to_underlying", "name": "__cpp_lib_to_underlying",
"values": { "c++2b": 202102 }, "values": { "c++2b": 202102 },
"headers": ["utility"], "headers": ["utility"],
}, { }, {
"name": "__cpp_lib_transformation_trait_aliases", "name": "__cpp_lib_transformation_trait_aliases",
"values": { "c++14": 201304 }, "values": { "c++14": 201304 },
▲ Show 20 Lines • Show All 493 Lines • Show Last 20 Lines
libcxx/test/std/language.support/support.limits/support.limits.general/charconv.version.pass.cpp