This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GOLD][PATCH] Add Arm_output_section class and downcasting hooks.


I committed the patch with changes you suggested.   I also added
asserts to make sure that group_end is never the end of the container.
 The asserts did not trigger in my testing.  So I think the original
code was correct but I change it anyway.  The assignment of
container.end() to group_end is mainly for eliminating possible
unitialized variable warnings.  When a new stub-group is beind built,
group_end never points to container.end().

As for the static casts,  I don't think I can reliably detects the
subclasses of Output_section, Output_relaxed_input_section and
Sized_relobj by checking the machine code and endianity of the target.
 At the moment it works because we always use these subclasses instead
of the bases so I do not worry too mcuh about that.

-Doug

2009/10/23 Doug Kwan (Ãö®¶¼w) <dougkwan@google.com>:
> The range is meant to be inclusive, if I sometimes pass container.end(),
> then it is a bug.  I will fix that may be with a do while loop.
>
> Originally I used static casts, they are actually safe because of context.
> In the ARM back-end, we always use the target subclass of Output_object for
> example.  I will change that back to static casts with some comments
> explaining why they are safe then.
>
> Thanks for looking at the patch.
>
> On Oct 23, 2009 6:44 PM, "Ian Lance Taylor" <iant@google.com> wrote:
>
> "Doug Kwan (Ãö®¶¼w)" <dougkwan@google.com> writes: >> 2009-10-22 Doug Kwan
> <dougkwan@google.com> >> ...
>
>> +  Input_section_list::const_iterator after_end = end + 1;
>> +  for (Input_section_list::const_iterator p = begin; p != after_end; ++p)
>
> This is not the convention for C++ iterators.  The convention is p =
> begin; p != end; ++p, without using after_end.  Moreover, I see that
> later on you do sometimes pass CONTAINER.end() as the end parameter
> here, in which case end + 1 is undefined.
>
>
>
>> +  // Return a pointer to self if this is an Arm_relobj or NULL otherwise.
>> +  template<bool big_endian>
>> +  Arm_relobj<big_endian>*
>> +  arm_relobj()
>> +  {
>> +    Arm_relobj<big_endian>* as_arm_relobj;
>> +    this->do_arm_relobj(&as_arm_relobj);
>> +    return as_arm_relobj;
>> +  }
>
> This is nicely type safe but I don't think we want target specific
> stuff getting into object.h/object.cc.  Just use a static_cast in
> arm.cc.  If you want to make it safe, do something which verifies that
> machine_code(), get_size(), and is_big_endian() are what you expect.
>
>
> This is OK with these changes if you only change arm.cc.  If
> static_cast doesn't do what you need, let's take another round.
>
> Ian
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]