Re: Mips target in gold - revision 2 - part 1

[Cary Coutant <ccoutant at google dot com>]

>> You shouldn't need to include "stringpool.h". The set_dynsym_indexes
>> function takes a Stringpool*, so you only need a forward declaration
>> for class Stringpool.
>
> Stringpool is defined as a template in file stringpool.h. If I remove "stringpool.h" then following needs to be added to file target.h:
>
> template<typename Stringpool_char>
> class Stringpool_template;
>
> typedef Stringpool_template<char> Stringpool;

Right. Putting the include in target.h is better.

dynsym.patch:

> 2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * symtab.cc (Symbol_table::set_dynsym_indexes): Allow a target to set
> dynsym indexes.
> * target.h (Target::has_custom_set_dynsym_indexes): New function.
> (Target::set_dynsym_indexes): New function.

+  // Whether the target has a custom set_dynsym_indexes method.
+  virtual bool
+  has_custom_set_dynsym_indexes() const
+  { return false; }
+
+  // Custom set_dynsym_indexes method for a target.
+  virtual unsigned int
+  set_dynsym_indexes(std::vector<Symbol*>*, unsigned int,
std::vector<Symbol*>*,
+                     Stringpool*, Versions*, Symbol_table*) const
+  { gold_unreachable(); }

These public interfaces should be changed to non-virtual functions
that call protected virtual functions (avoid public virtual
interfaces).  The virtual functions typically are named "do_...".
Check most of the other public interfaces in this class for examples.
(For rationale, see the discussion of the NVI idiom in Item 35 in
Scott Meyers' _Effective C++_.)


dynamic-tag.patch:

>  2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * output.cc (Output_data_dynamic::Dynamic_entry::write):
> Get the value of DYNAMIC_CUSTOM dynamic entry.
> * output.h (Output_data_dynamic::add_custom): New function.
> (Dynamic_entry::Dynamic_entry): New constructor for DYNAMIC_CUSTOM
> dynamic entry.
> (enum Dynamic_entry::Classification): Add DYNAMIC_CUSTOM.
> * target.h (Target::dynamic_tag_custom_value): New function.

This is OK.


adjust_dynsym.patch:

>  2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * symtab.cc (Symbol_table::sized_write_globals): Allow a target to
> adjust dynamic symbol value.
> * target.h (Target::adjust_dyn_symbol): New function.

+  // Adjust the value written to the dynamic symbol table.
+  virtual void
+  adjust_dyn_symbol(const Symbol*, unsigned char*) const
+  { }

Change to a non-virtual public interface, as above.


init-output-data.patch:

>  2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * symtab.cc (Sized_symbol<32>::init_output_data):
> Instantiate the template.
> (Sized_symbol<64>::init_output_data): Likewise.

This is OK.


nonvis.patch:

>  2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * symtab.h (Symbol::set_nonvis): New function.

This is OK.

Thanks!

-cary