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

Jeffrey Yasskin jyasskin at google.com
Fri Aug 7 14:12:15 PDT 2009


On Fri, Aug 7, 2009 at 2:09 PM, Chris Lattner<clattner at apple.com> wrote:
>
> 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!

Will do. :)




More information about the llvm-commits mailing list