[llvm] r204431 - [Constant Hoisting] Replace the MapVector with a separate Map and Vector to keep track of constant candidates.

[David Blaikie]

On Tue, Mar 25, 2014 at 11:07 AM, Juergen Ributzka <juergen at apple.com> wrote:
> Ideally it shouldn't be allowed, but unfortunatelly it isn't very convenient
> to use const access methods. I guess that is the reason it wasn't
> implemented that way.

Yep - but it might be a necessary/appropriate limitation (std::map
makes this choice too - though it's annoying mostly when ordering
(especially likely when ordering by an external comparator) doesn't
match with constness (eg: only a subset of fields are considered in
the ordering yet those not considered are still, needlessly,
immutable)).

> On Mar 21, 2014, at 1:54 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> Is MapVector's API broken by exposing the ability to break its
> invariants like this? Seems like it really shouldn't allow non-const
> access to the keys... especially since they're duplicated in the map
> and vector so they won't be kept in sync.
>
> Perhaps it'd be nice to fix it, so this kind of confusing code can't be
> written?


> >If someone later on modifies the code and doesn't know or
> > missed the fact that the mapping is no longer valid, then they may use the
> > MapVector incorrectly.
>
> Would it be useful to invalidate the map and/or vector when this sort
> operation occurs? Perhaps emptying both the map and vector (simply
> std::move the vector into a local, sort it, use it, then leave the
> function with both map and vector empty - since leaving them non-empty
> is just a trap for someone later?)

& this ^?