This is an archive of the discontinued LLVM Phabricator instance.

[libc++][NFC] Replace _LIBCPP_STD_VER > x with _LIBCPP_STD_VER >= x
ClosedPublic

Authored by philnik on Feb 13 2023, 4:05 PM.

Details

Summary

This change is almost fully mechanical. The only interesting change is in generate_feature_test_macro_components.py to generate _LIBCPP_STD_VER >= instead. To avoid churn in the git-blame this commit should be added to the .git-blame-ignore-revs once committed.

Diff Detail

Event Timeline

philnik created this revision.Feb 13 2023, 4:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2023, 4:05 PM
philnik requested review of this revision.Feb 13 2023, 4:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2023, 4:05 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
var-const accepted this revision as: var-const.Feb 13 2023, 4:30 PM

LGTM, but I'll leave the approval to @ldionne (I remember this was discussed before and want to make sure this reflects the final decision; personally, I fully support this and think it's long overdue).

jloser added a subscriber: jloser.Feb 13 2023, 4:35 PM
jloser added inline comments.
libcxx/utils/generate_feature_test_macro_components.py
286

Out of curiosity, did we decide (or should we) to do the same for TEST_STD_VER? It seems slightly out of place seeing it use >. WDYT?

ldionne accepted this revision.Feb 15 2023, 7:22 AM
ldionne added inline comments.
libcxx/utils/generate_feature_test_macro_components.py
286

I agree, however that touches even more files. Let's do it separately if at all.

This revision is now accepted and ready to land.Feb 15 2023, 7:22 AM

Please commit this ASAP to avoid conflicts

This revision was landed with ongoing or failed builds.Feb 15 2023, 7:52 AM
This revision was automatically updated to reflect the committed changes.

LGTM, but I'll leave the approval to @ldionne (I remember this was discussed before and want to make sure this reflects the final decision; personally, I fully support this and think it's long overdue).

AFAIK we never made a decision :-(

I really would like to discuss this upfront instead of rushing it in. I expect merge conflicts in is some of my formatting patches. Not hard to fix, but it's quite annoying.

LGTM, but I'll leave the approval to @ldionne (I remember this was discussed before and want to make sure this reflects the final decision; personally, I fully support this and think it's long overdue).

AFAIK we never made a decision :-(

I really would like to discuss this upfront instead of rushing it in. I expect merge conflicts in is some of my formatting patches. Not hard to fix, but it's quite annoying.

Mark and I talked about this offline. So my recollection is that we did have a discussion at some point on a review (or on Discord) where we had agreed to do this. I think I was initially against but had ended up agreeing since most folks wanted to do it. Clearly, this wasn't unanimous.

Mark and I agreed to set up an infrequent meeting with regular libc++ contributors to discuss these sorts of broad "policy" decisions in the future to make sure we had a clear agreement on those beforehand. We would take a few notes and publish them to Discourse or add them to a design document of sorts in the repository, making sure that those decisions are recorded somewhere.