This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Replace _VSTD with std in __ranges/
ClosedPublic

Authored by philnik on Feb 10 2022, 4:01 PM.

Diff Detail

Event Timeline

philnik requested review of this revision.Feb 10 2022, 4:01 PM
philnik created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2022, 4:01 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Quuxplusone added 1 blocking reviewer(s): ldionne.Feb 11 2022, 7:32 AM

Assuming this is a completely mechanical sed -i -e s/_VSTD::/std::/g ../libcxx/include/__ranges/*.h, and assuming that you'll actually redo the sed immediately before pushing it (to avoid any merge/rebase conflicts), this seems technically fine to me.
My impression is that we absolutely don't want to do this kind of replacement on e.g. __algorithm/, __memory/, etc, because that would annoy downstream vendors; but perhaps __ranges/ is so new that it's a special case? Therefore I defer to @ldionne on this more-political-than-technical question.

Mordante resigned from this revision.Feb 11 2022, 9:29 AM

LGTM from my side especially since there were no objections on Discord for the change to ranges.
However since I've no patches for ranges that can be conflicting I leave approval to those who work actively in the ranges code.

ldionne accepted this revision.Feb 14 2022, 12:48 PM

LGTM, but please make sure you re-run the sed before committing, in case there were race conditions.

Also, I looked at the patch and that seems to be the case, but I am assuming that this is purely a change from _VSTD:: to std:: (and so technically we could make this patch a no-op by reverting to _VSTD::). If that's not the case, please call it out.

This revision is now accepted and ready to land.Feb 14 2022, 12:48 PM
This revision was landed with ongoing or failed builds.Feb 14 2022, 3:39 PM
This revision was automatically updated to reflect the committed changes.