Doug,

Once again, thanks for the review!

On 08/15/2015 07:08 AM, Doug Evans wrote:
>> +/* Implement the struct symbol_block_ops::get_frame_base method.  */
>> +
>> +static CORE_ADDR
>> +block_op_get_frame_base (struct symbol *framefunc, struct frame_info *frame)
>> +{
>> +  if (SYMBOL_BLOCK_OPS (framefunc)->find_frame_base_location != NULL)
>
> ====
> It's hard for this reader to figure out why this test is here.
> If the code gets here, when would find_frame_base_location be NULL?
> Seems like we could just assert find_frame_base_location is non-NULL.

Good catch! This is fixed, now, with a comment explaining this.

>> +  /* Old compilers may not provide a static link, or they may provide an
>> +     invalid one.  For such cases, fallback on the old way to evaluate
>> +     non-local references: just climb up the call stack and pick the first
>> +     frame that contains the variable we are looking for.  */
>> +  if (frame == NULL)
>> +    {
>> +      frame = block_innermost_frame (var_block);
>> +      if (!frame)
>
> ====
> frame == NULL

Done.

> Ok with them fixed (not sure what the right fix for the first one is
> though, I could be missing something).

As said in my previous mail, I'm not sure I want to commit it: I tried 
to investigate this 4% unexpected performance drop, at least to see what 
happens with a profiler but I could not reproduce.

-- 
Pierre-Marie de Rodat