[llvm-commits] [PATCH] Add a ValueMap type, whose keys act like WeakVHs

[Jeffrey Yasskin]

There's a new and more correct patch at
http://codereview.appspot.com/download/issue132055_1002.diff. I made 3
changes:

1. Support const keys.
2. Support a lock around map changes from the CallbackVH. The
ExecutionEngine needs this since the CallbackVH can be fired from a
place the user doesn't really control.
3. Fix a bug involving callbacks that call back into the ValueMap.

This version passes tests as a replacement for the CallbackVHs in
http://codereview.appspot.com/133043/show.

On Sat, Oct 17, 2009 at 7:52 PM, Jeffrey Yasskin <jyasskin at google.com> wrote:
> On Sat, Oct 17, 2009 at 4:05 PM, Chris Lattner <clattner at apple.com> wrote:
>>
>> On Oct 17, 2009, at 12:42 AM, jyasskin at gmail.com wrote:
>>>
>>> I guess I'll have to figure out how to fix it before submitting.
>>
>> I'll review it when it is fixed.
>
> I've fixed lit.py in r84387. This patch didn't need to change.
>
>>> Description:
>>> ValueMap<Value*, T> is safe to use even when keys get RAUWed and
>>> deleted
>>> during its lifetime. By default the keys act like WeakVHs, but users
>>> can
>>> pass a third template parameter to configure how updates work and
>>> whether to do anything beyond updating the map on each action.
>>>
>>> This type should make http://codereview.appspot.com/133043/diff/1/2
>>> quite a bit simpler, but I haven't yet updated it so I could have
>>> missed
>>> something.
>>
>> Sounds nice, what happens when X->RAUW(Y) and both X and Y are already
>> in the map?
>
> Right now, you wind up with Y mapped to X's old target, and Y's old
> target lost. ValueMapTest.DefaultCollisionBehavior tests for this. It
> should be easy to add the ability to configure this behavior
> (Config::Merge(ValueT& OldTarget, const ValueT& NewTarget), maybe with
> extra parameters), but I didn't need it for my other changes.
>