Hi both, PTAL. I was surprised to find out that both std::variant<int> and std::optional<int> have a sizeof 12 in my environment, this is my attempt to fix it. Please do let me know if there are other places I need to change, or how I would write a test to prevent future regressions.

LGTM, no concerns from my side.

Late LGTM.

"Jan Wilken Dörrie <jdoerrie@google.com>" works for me :)

Thanks, Louis! I rebased the diff on the latest master. Could you land the patch for me? I believe I lack the required permissions to do this myself.

Rebase

In D70343#2034645, @palmer wrote:

Here are some binary size numbers, from a Chromium release build. Here is the build configuration (likely meaningful only to Chromium people; but this basically means "fully optimized except for the final PGO phase"):

~/chromium/src $ cat out/Release/args.gn
is_debug = false
is_official_build = true
is_asan = false
use_goma = true
enable_nacl = false
is_component_build = false

origin/master:
-rwxr-xr-x 1 palmer primarygroup 162973344 May 12 17:30 out/Release/chrome

With libcxx-debug-level-0:
-rwxr-xr-x 1 palmer primarygroup 169825792 May 12 18:20 out/Release/chrome
Cost: 6,852,448 (4.2% of 162,973,344)

With libcxx-debug-level-0 with __builtin_trap instead of printing:
-rwxr-xr-x 1 palmer primarygroup 167307088 May 13 10:08 out/Release/chrome
Cost: 4,333,744 (2.6% of 162,973,344)

So the cost is still high, but significantly less with __builtin_trap. So I think this mode is valuable.

LGTM, but I would still like to know whether we actually need this in addition to _LIBCPP_DEBUG=0. As far as I can tell setting _LIBCPP_DEBUG to 0 makes _LIBCPP_ASSERT(x, m) be equivalent to ((x) ? (void)0 : abort_fun(x, m)), where abort_fun is effectively the following:

Oh interesting! Yes, it looks like you are right.

Friendly ping

Friendly Ping

In D70343#1748522, @EricWF wrote:

What's the difference between _LIBCPP_HARDEN and simply defining _LIBCPP_DEBUG=0? That should turn on _LIBCPP_ASSERT in the same cases.

Addressed Marshall's comments

Thanks Jorg! Given that the status is still "Needs Review", I assume I also still need an LGTM from Marshall, Louis or Eric, right?

Friendly Ping :)

Full patch

Formatting

In D60069#1449979, @mclow.lists wrote:

I'm less enthusiastic about this change than the one for PR39871, because there we were being inconsistent with ourselves.
However, my lack of enthusiasm is no reason not to land this.

Remove redundant public: access controls

In D60069#1449932, @mclow.lists wrote:

In D60069#1449928, @mclow.lists wrote:

Did you check the places that inherit from tuple_element? The public/private bits change between class and struct.

Never mind. I was thinking of something else; I don't think that anything inherits from tuple_element

Thanks for applying a fix based on my patch! I could have sworn the added tests failed locally before applying the algorithm patch, though... oh well.

Ping, please take a look at this.