This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] cleans up __cpp_concepts mess
ClosedPublic

Authored by cjdb on Feb 18 2021, 5:59 PM.

Details

Summary

libc++ was previously a bit confused by what the value of __cpp_concepts
should be. Also replaces __floating_point with floating_point now
that it exists.

Diff Detail

Event Timeline

cjdb created this revision.Feb 18 2021, 5:59 PM
cjdb requested review of this revision.Feb 18 2021, 5:59 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 18 2021, 5:59 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
miscco accepted this revision.Feb 18 2021, 10:37 PM

Looks good to me

*Insert obligatory rant about double negation*

miscco added inline comments.Feb 18 2021, 10:39 PM
libcxx/test/std/language.support/support.limits/support.limits.general/version.version.pass.cpp
2886

Could we clean that up to use!__LIBCPP_HAS_NO_CONCEPTS

ldionne accepted this revision.Feb 19 2021, 8:32 AM

Thanks!

This revision is now accepted and ready to land.Feb 19 2021, 8:32 AM

It seems this fails to build on Apple due to an issue in _LIBCPP_HAS_NO_CONCEPTS. I'll fix that in main this weekend.

cjdb updated this revision to Diff 325016.Feb 19 2021, 9:32 AM

applies @miscco's fix

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

🤦Done.

1a5c92f68021 should fix this patch. Can you rebase on main to see whether the CI passes for this patch?

cjdb updated this revision to Diff 325494.Feb 22 2021, 10:59 AM

rebases to activate CI

Mordante added inline comments.Feb 22 2021, 11:25 AM
libcxx/include/concepts
152

I see !_LIBCPP_HAS_NO_CONCEPTS instead of !defined(_LIBCPP_HAS_NO_CONCEPTS) at several places in this patch. Probably part of the new build failures.

cjdb updated this revision to Diff 326461.Feb 25 2021, 11:49 AM

fixes silly preprocessor mistake

ldionne accepted this revision.Feb 25 2021, 1:48 PM
ldionne added inline comments.
libcxx/include/numbers
103–115

Assuming you didn't change the value of these constants :-)

cjdb updated this revision to Diff 326577.Feb 25 2021, 6:36 PM
This comment was removed by cjdb.
This revision was automatically updated to reflect the committed changes.

@cjdb, after this landed and then @zoecarver's D60368 landed on top of it and maybe merge-conflicted with it... I'm now seeing that the feature-test-macro tests that are checked in do not match the output you get by running the libcxx/utils/generate_feature_test_macro_components.py script. Could you (@cjdb) run libcxx/utils/generate_feature_test_macro_components.py locally, check whether it matches what you intended, and if so, check in those changes? (If the generated output doesn't match what you intended, then probably you should post another PR.)

cjdb added a comment.Mar 3 2021, 6:12 PM

@cjdb, after this landed and then @zoecarver's D60368 landed on top of it and maybe merge-conflicted with it... I'm now seeing that the feature-test-macro tests that are checked in do not match the output you get by running the libcxx/utils/generate_feature_test_macro_components.py script. Could you (@cjdb) run libcxx/utils/generate_feature_test_macro_components.py locally, check whether it matches what you intended, and if so, check in those changes? (If the generated output doesn't match what you intended, then probably you should post another PR.)

Nice catch! Looking at the diff here, it appears that I didnt' change libcxx/utils/generate_feature_test_macro_components.py properly. Making a patch for it now.