[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

> One way we could deal with this is by adding an attribute to the compiler to indicate "the const is a lie", that we can apply to `std::pair::first`, with the semantics being that a top-level `const` is ignored when determining the "real" type of the member (and so mutating the member after a `const_cast` has defined behavior).  This would apply to //all// `std::pair`s, not just the `node_handle` case, but in practice we don't optimize on the basis of a member being declared `const` *anyway*, so this isn't such a big deal right now.

I'm a fan of this idea, this would also have the bonus of fixing some pre-existing type-punning between `pair<const K, V>` and `pair<K, V>` (see `__hash_value_type` and `__value_type` in `<unordered_map>` and `<map>`).

> Another possibility would be a compiler intrinsic to change the type of the pair, in-place, from a `std::pair<const K, V>` to a `std::pair<K, V>`, without changing anything about the members -- except that the `first` member changes type.

How could this work? It would still be possible for TBAA to assume that `void m(std::pair<const K, V>*, std::pair<K, V>*);`'s parameters don't alias when optimizing `m`, regardless of how we form the non-const pair, right?

> Yet another possibility would be to only ever access the `key` field through a type annotated with `__attribute__((may_alias))`, and then launder the node pointer when we're ready to insert it back into the container (to "pick up" the new value of the const member). That's formally breaking the rules (a `launder` isn't enough, because we didn't actually create a new object, and in any case we still mutated the value of a `const` subobject), but it'll probably work fine across compilers that support the attribute.

I think we could fall back to this in cases where the attribute from idea (1) isn't available. There, we could mark std::pair as may_alias and still fix `__hash_value_type` and `__value_type`. This would pessimize std::pair, but only for "older" compilers.

So unless I've gotten this all wrong, I'll write a patch to support that new attribute and come back to this patch if/when that lands. Sound good?

Thanks for the feedback!
Erik


https://reviews.llvm.org/D46845