[PATCH] Cleaning up remaining static initializers in Signals.inc

Duncan Exon Smith dexonsmith at apple.com
Tue Sep 2 16:54:31 PDT 2014


LGTM. 

-- dpnes

> On Sep 2, 2014, at 7:22 PM, Chris Bieneman <beanz at apple.com> wrote:
> 
> How is this?
> 
> -Chris
> <Signals.diff>
> 
>> On Sep 2, 2014, at 3:53 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> 
>>> On 2014 Sep 2, at 18:29, Chris Bieneman <beanz at apple.com> wrote:
>>> 
>>> This patch cleans up the remaining static initializers in Signals.inc by converting them to ManagedStatic objects.
>>> 
>>> Phabricator review:
>>> http://reviews.llvm.org/D5159
>>> 
>>> Thanks,
>>> -Chris
>> 
>> In a couple of cases, such as...
>> 
>>> @@ -182,8 +184,8 @@
>>>  }
>>> 
>>>  // Otherwise if it is a fault (like SEGV) run any handler.
>>> -  for (unsigned i = 0, e = CallBacksToRun.size(); i != e; ++i)
>>> -    CallBacksToRun[i].first(CallBacksToRun[i].second);
>>> +  for (unsigned i = 0, e = CallBacksToRun->size(); i != e; ++i)
>>> +    (*CallBacksToRun)[i].first((*CallBacksToRun)[i].second);
>>> 
>>> #ifdef __s390__
>>>  // On S/390, certain signals are delivered with PSW Address pointing to
>> 
>> ... you're doing an expensive dereference inside a loop.  I'd prefer you
>> saved the reference at the first dereference in each function and reused
>> it.
>> 
>> LGTM with that change.
> 



More information about the llvm-commits mailing list