This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold patch] incremental 5/18: support --start-lib/--end-lib
- From: Ian Lance Taylor <iant at google dot com>
- To: Cary Coutant <ccoutant at google dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Tue, 29 Mar 2011 17:24:26 -0700
- Subject: Re: [gold patch] incremental 5/18: support --start-lib/--end-lib
- References: <BANLkTikMgD5ZVCJ8kR4_bvkUmGU98PDCKQ@mail.gmail.com>
Cary Coutant <ccoutant@google.com> writes:
> I've got a series of 14 more patches lined up now for incremental
> linking support, including the four that are still pending from last
> September. I've updated the pending patches a bit and rebased them to
> the current head. I'll send them to you a few at a time.
Just checking--are you going to resend the patches from last September?
(I think there are only three, as one was approved.)
> 2011-03-28 Cary Coutant <ccoutant@google.com>
>
> * archive.cc (Library_base::should_include_member): Move
> method here from class Archive.
> (Archive::Archive): Initialize base class.
> (Archive::should_include_member): Move to base class.
> (Archive::do_for_all_unused_symbols): New function.
> (Add_archive_symbols::run): Remove redundant access to
> incremental_inputs.
> (Lib_group::Lib_group): Initialize base class.
> (Lib_group::do_filename): New function.
> (Lib_group::include_member): Pass pointer to Lib_group to
> report_object.
> (Lib_group::do_for_all_unused_symbols): New function.
> (Add_lib_group_symbols::run): Report archive information for
> incremental links.
> * archive.h (class Library_base): New base class.
> (class Archive): Derive from Library_base.
> (Archive::filename): Move to base class.
> (Archive::set_incremental_info): Likewise.
> (Archive::incremental_info): Likewise.
> (Archive::Should_include): Likewise.
> (Archive::should_include_member): Likewise.
> (Archive::Armap_entry): Remove.
> (Archive::Unused_symbol_iterator): Remove.
> (Archive::unused_symbols_begin): Remove.
> (Archive::unused_symbols_end): Remove.
> (Archive::do_filename): New function.
> (Archive::do_get_mtime): New function.
> (Archive::do_for_all_unused_symbols): New function.
> (Archive::task_): Move to base class.
> (Archive::incremental_info_): Likewise.
> (class Lib_group): Derive from Library_base.
> (Lib_group::do_filename): New function.
> (Lib_group::do_get_mtime): New function.
> (Lib_group::do_for_all_unused_symbols): New function.
> (Lib_group::task_): Move to base class.
> * dynobj.cc (Sized_dynobj::do_for_all_global_symbols): New
> function.
> * dynobj.h (Sized_dynobj::do_for_all_global_symbols): New
> function.
> * incremental.cc (Incremental_inputs::report_archive_begin):
> Use Library_base; call library's get_mtime; add incremental inputs
> entry before members.
> (class Unused_symbol_visitor): New class.
> (Incremental_inputs::report_archive_end): Use Library_base; use
> visitor class to record unused symbols; don't add incremental inputs
> entry after members.
> (Incremental_inputs::report_object): Use Library_base.
> * incremental.h
> (Incremental_archive_entry::Incremental_archive_entry): Remove
> unused Archive parameter.
> (Incremental_inputs::report_archive_begin): Use Library_base.
> (Incremental_inputs::report_archive_end): Likewise.
> (Incremental_inputs::report_object): Likewise.
> * object.cc (Sized_relobj::do_for_all_global_symbols): New
> function.
> * object.h (Object::for_all_global_symbols): New function.
> (Object::do_for_all_global_symbols): New function.
> (Sized_relobj::do_for_all_global_symbols): New function.
> * plugin.cc (Sized_pluginobj::do_for_all_global_symbols): New
> function.
> * plugin.h (Sized_pluginobj::do_for_all_global_symbols): New
> function.
> +// Iterate over all unused symbols, and call the visitor class V for each.
> +
> +void
> +Archive::do_for_all_unused_symbols(Symbol_visitor_base& v) const
> +{
> + for (std::vector<Armap_entry>::const_iterator p = this->armap_.begin();
> + p != this->armap_.end();
> + ++p)
> + {
> + if (this->seen_offsets_.find(p->file_offset)
> + == this->seen_offsets_.end())
> + v.visit(this->armap_names_.data() + p->name_offset);
> + }
> +}
In gold we should avoid non-const references. Make v a pointer instead.
> + // Abstract base class for processing unused symbols.
> + class Symbol_visitor_base
> + {
> + public:
> + Symbol_visitor_base()
> + { }
> +
> + virtual
> + ~Symbol_visitor_base()
> + { }
> +
> + virtual void
> + visit(const char*) = 0;
> + };
The visit member function should have a comment saying that the
parameter is.
> + // Iterator for unused global symbols in the library.
> + void
> + for_all_unused_symbols(Symbol_visitor_base& v) const
> + { this->do_for_all_unused_symbols(v); }
Clarify: unused symbol definitions or unused symbol references?
> +// Iterate over global symbols, calling a visitor class V for each.
s/global symbol/global defined symbols/
> +// Visitor class for processing the unused global symbols in a library.
> +
> +class Unused_symbol_visitor : public Library_base::Symbol_visitor_base
> +{
Expand comment: what sort of processing?
> diff --git a/gold/object.cc b/gold/object.cc
> index e2c0113..954961a 100644
> --- a/gold/object.cc
> +++ b/gold/object.cc
> @@ -1691,6 +1691,31 @@ Sized_relobj<size, big_endian>::do_should_include_member(Symbol_table* symtab,
> return Archive::SHOULD_INCLUDE_UNKNOWN;
> }
>
> +// Iterate over global symbols, calling a visitor class V for each.
> +
> +template<int size, bool big_endian>
> +void
> +Sized_relobj<size, big_endian>::do_for_all_global_symbols(
> + Read_symbols_data* sd,
> + Library_base::Symbol_visitor_base& v)
> +{
s/global symbols/global defined symbols/
This is OK with those changes.
Thanks.
Ian