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

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

In https://reviews.llvm.org/D46845#1098634, @rsmith wrote:

> 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.


Would you mind elaborating on this? If we don't optimize on a member being const, then what should this attribute actually do? The only thing I can think of is making a field with type `const T` with the attribute claim it's a `T` to TBAA, but that would be a no-op because loads and stores through those types are compatible anyways.

In https://reviews.llvm.org/D46845#1098634, @rsmith wrote:

> 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.


What about instead of type-punning between pair<const K, V> and pair<K, V> (with may_alias) we just did a const_cast in key, and launder the pair when finished? Because loads and stores to `const K` and `K` are already assumed to alias each other, we should be able to omit the may_alias attribute (right?). Would that be enough to placate the compiler? If it is, then I think we should just do this and avoid adding another attribute to clang. I'll upload a patch with this to show what I mean.

Thanks! 
Erik


https://reviews.llvm.org/D46845