[llvm-commits] [PATCH] Use AssertingVH for the GlobalValue maps

Chris Lattner clattner at apple.com
Fri Aug 7 14:09:29 PDT 2009


On Aug 7, 2009, at 1:00 PM, Jeffrey Yasskin wrote:

> On Fri, Aug 7, 2009 at 9:44 AM, Chris Lattner<clattner at apple.com>  
> wrote:
>>
>> On Aug 6, 2009, at 11:04 AM, jyasskin at gmail.com wrote:
>>
>>> Reviewers: llvm-commits_cs.uiuc.edu,
>>>
>>> Description:
>>> To catch bugs like the one fixed in
>>> http://llvm.org/viewvc/llvm-project?view=rev&revision=78127, I'm
>>> changing the ExecutionEngine's global mappings to hold
>>> AssertingVH<const
>>> GlobalValue>. That way, if unregistering a mapping fails to actually
>>> unregister it, we'll get an assert. Running the jit nightly tests
>>> doesn't uncover any actual instances of the problem.
>>
>> The patch looks ok, but please rename AsValue -> GetAsValue or
>> getAsValue.
>
> Done and committed. Thanks!
>
>>  Also, please send diffs/patches so I don't have to click
>> through silly web apps :)
>
> Do you want me to always attach the patch or just link directly to the
> rietveld patch:
> http://codereview.appspot.com/download/issue104054_1.diff ?

Ah, linking to the patch is fine.  You can put in both links, I just  
didn't see an obvious way to get "one big diff".  Thanks!

-Chris



More information about the llvm-commits mailing list