Sorry, didn't see this earlier as this drowned in my email filters :/

In D81770#2102480, @minjaehwang wrote:

clang-tidy complains that "benchmark/benchmark.h" is not found at CartesianBehcmarks.h. This diff does not modify that file and this is a benign error. It compiles without a problem.

Could anyone land this diff into the master?

In D78763#2104284, @mvels wrote:

I'm going to abandon this diff, seems arcanist, phabricator and I can't see each other eye to eye...... :\ which makes me sad,....

Create new diff (same as this) in https://reviews.llvm.org/D82220

I'm going to abandon this diff, seems arcanist, phabricator and I can't see each other eye to eye...... :\ which makes me sad,....

arc diff with --head=HEAD 'HEAD~1'

Running arc diff --update D78763

Looks like harbormaster builds use arcanist, and arcanist has **magic ** making awful diffs?

I'm mystified why the build fails, it fails on applying the patch:

Uploading clean diff

Uploaded clean diff in case harbormaster is confused about diffs

Thanks for the reviews!

Optimed ternary op for ascizz string -> always evalutate length in inlined constant string.

Cleaned up const char* ternary, evaluating traits_type::length for all constant strings

Removed __config changes

Revamped

Fixed __config comment

Fixed NOEXCEPT comments

renamed end --> pos

Update diff after master pull

Removed Apple V2 abi list

White space / comment clean up

Fixed AsanTransaction for single 'construct_at' use case

Well, I think we should probably remove that notion, or merge it with the "unstable" ABI. It's not really a thing since V2 is basically unstable AFAICT.

Just fix it and we can improve the situation (i.e. probably remove it) later.

I have no way to verify / test this (cross compiling apple darwin v2 looks to be impossible?), and guessing the mangled names from x86 compiles seems dubious as well given the diffs I see on apple v1 vs linux v1

1. Pass this->__end_ to the helper functions
2. Return the new end pointer from those helpers

I am skeptical why step 2 is needed at all. You never remove setting of this->__end_. So why do you need to do work that has already been done? Could you please verify that the second part is indeed necessary?

If it is indeed necessary I note that you pessimize the slow path by decrementing and then incrementing.

I would greatly prefer it if you would directly return in both the fast and the slow path.

They are removed from the unstable ABI, but not from the ABI v2. Those are different things (and I guess it doesn't really make sense for them to be different). But we should either decide that we don't test abi lists for ABI v2, or fix it. I suggest we fix it for now in order to make progress on this. You just need to update the ABI list for v2, it shouldn't be difficult.

Restricted assign to __min_cap

In D78763#2001599, @ldionne wrote:

Please make sure you update the ABI v2 abi list when you make these changes. It's currently broken (and this change will break it even more). You can run make check-cxx-abilist to see the failures.

Or, I guess we can also have a discussion to disable the abi list tests for unstable ABIs, since it's not that useful I guess.

Fixed external instantiations for unstable (removed __resize)

Simplified change to inlined optimizations.

Added proper constant for min short -> long promotion size

Adjusted for sizeof(value_type)

New diff after rebase

Thanks, arc is not my friend :\ Fixed the diff using --base=git:origin

Thanks, arc is not my friend :\ Fixed the diff using --base=git:origin

BTW, you could argue 'arc' was smart enough to recognize this was a rolled forward revert, and only diff against the original submit, alas :)

Diff against

Diff to HEAD

Fixed bug / omitting __n parameter

Rebased

• Moved test

Diff vs origin

Updated comments, synced with master

Update comments

Heads up: this trivial change was per-approved per EricWf, I will be committing after double checking build and unit tests for stable and unstable