Re: [gold][patch] Add the --start-lib end --end-lib options

[Ian Lance Taylor <iant at google dot com>]

Rafael Espindola <espindola@google.com> writes:

> 2010-03-17   Rafael Espindola  <espindola@google.com>
>
> 	* archive.cc (Should_include): Move to archive.h.
> 	(should_include_member): Make it a member of Archive.
> 	(Lib_group): New.
> 	(Add_lib_group_symbols): New.
> 	* archive.h: Include options.h.
> 	(Archive_member): Moved from Archive.
> 	(Should_include): Moved from archive.cc.
> 	(Lib_group): New.
> 	(Add_lib_group_symbols): New.
> 	* dynobj.cc (do_should_include_member): New.
> 	* dynobj.h (do_should_include_member): New.
> 	* gold.cc (queue_initial_tasks): Update call to queue.
> 	* main.cc (main): Print lib group stats.
> 	* object.cc (do_should_include_member): New.
> 	* object.h: Include archive.h.
> 	(Object::should_include_member): New.
> 	(Object::do_should_include_member): New.
> 	(Sized_relobj::do_should_include_member): New.
> 	* options.cc (General_options::parse_start_lib): New.
> 	(General_options::parse_end_lib): New.
> 	(Input_arguments::add_file): Handle lib groups.
> 	(Input_arguments::start_group): Check we are not in a lib.
> 	(Input_arguments::start_lib): New.
> 	(Input_arguments::end_lib): New.
> 	* options.h (General_options): Add start_lib and end_lib.
> 	(Input_argument::lib_): New.
> 	(Input_argument::lib): New.
> 	(Input_argument::is_lib): New.
> 	(Input_file_lib): New.
> 	(Input_arguments::in_lib_): New.
> 	(Input_arguments::in_lib): New.
> 	(Input_arguments::start_lib): New.
> 	(Input_arguments::end_lib_): New.
> 	* plugin.cc (Pluginobj::get_symbol_resolution_info): Mark symbols
> 	in unused members as preempted.
> 	(Sized_pluginobj::do_should_include_member): New.
> 	* plugin.h (Sized_pluginobj::do_should_include_member): New.
> 	* readsyms.cc (Read_symbols::locks): If we are just reading a member,
> 	return the blocker.
> 	(Read_symbols::do_whole_lib_group): New.
> 	(Read_symbols::do_lib_group): New.
> 	(Read_symbols::do_read_symbols): Handle lib groups.
> 	(Read_symbols::get_name): Handle lib groups.
> 	* readsyms.h (Read_symbols): Add an archive member pointer.
> 	(Read_symbols::do_whole_lib_group): New.
> 	(Read_symbols::do_lib_group): New.
> 	(Read_symbols::member_): New.
> 	* script.cc (read_input_script): Update call to queue_soon.


> -	  if (t == SHOULD_INCLUDE_NO || t == SHOULD_INCLUDE_YES)
> +	  if (t == Archive::SHOULD_INCLUDE_NO ||
> +              t == Archive::SHOULD_INCLUDE_YES)
>  	    this->armap_checked_[i] = true;

Move the "||" to the start of the second line of the conditional.


> +          // Skip files with no symbols. Plugin objects have
> +          // member.sd_ == NULL

Period at end of sentence.

> +          if (obj != NULL &&
> +              (member.sd_ == NULL || member.sd_->symbol_names != NULL))

Move "&&" to start of second line of conditional.

> +              if (member.sd_)
> +                delete member.sd_;

Use an explicit member.sd_ != NULL.

> --- a/gold/archive.h
> +++ b/gold/archive.h
> @@ -28,6 +28,7 @@
>  
>  #include "fileread.h"
>  #include "workqueue.h"
> +#include "options.h"

I don't see why this #include is necessary.  Please don't add it if it
is not needed.


> +// Include a lib group member in the link.
> +void
> +Lib_group::include_member(Symbol_table* symtab, Layout* layout,
> +			  Input_objects* input_objects,
> +			  const Archive_member& member)
> +{

Add a blank line after the comment.


> +// This class represents the files surrunded by a --start-lib ... --end-lib.
> +
> +class Lib_group
> +{
> + public:
> +  Lib_group(const Input_file_lib* lib, Task* task)
> +      : lib_(lib), task_(task), members_()
> +  {
> +    members_.resize(lib->size());
> +  }

Write this->members_.

> +class Add_lib_group_symbols : public Task
> +{

This class needs a comment.

> +template<int size, bool big_endian>
> +Archive::Should_include
> +Sized_relobj<size, big_endian>::do_should_include_member(
> +				  Symbol_table* symtab, Read_symbols_data* sd,
> +                                  std::string* why)
> +{

When the first parameter is on a new line, it should be indented only
four spaces, and each parameter should be on a separate line.

> +void
> +General_options::parse_start_lib(const char*, const char*,
> +                                 Command_line* cmdline)
> +{
> +  cmdline->inputs().start_lib(cmdline->position_dependent_options());
> +}
> +
> +void
> +General_options::parse_end_lib(const char*, const char*,
> +                               Command_line* cmdline)
> +{
> +  cmdline->inputs().end_lib();
> +}
> +
> +

Only add one blank line here.

> @@ -1193,6 +1216,31 @@ Input_arguments::end_group()
>    this->in_group_ = false;
>  }
>  
> +
> +// Start a lib.
> +
> +void
> +Input_arguments::start_lib(const Position_dependent_options& options)
> +{

Only one blank line before "Start a lib" comment.

> +
> +  if (size_t(nsyms) > this->symbols_.size())
> +    {

Write static_cast<size_t>(nsyms).


>  void
> -Read_symbols::locks(Task_locker*)
> +Read_symbols::locks(Task_locker* tl)
>  {
> +  if(this->member_)
> +    tl->add(this, this->next_blocker_);

Add space after "if".  Write an explicit != NULL.


> @@ -246,6 +338,13 @@ Read_symbols::do_read_symbols(Workqueue* workqueue)
>            // We are done with the file at this point, so unlock it.
>            obj->unlock(this);
>  
> +          if (this->member_)
> +          {

Write an explicit this->member_ != NULL.

> +      if (this->member_)
> +        {

Likewise.

> -	       Input_group* input_group, Task_token* this_blocker,
> -	       Task_token* next_blocker)
> +	       Input_group* input_group, Archive_member *member,
> +               Task_token* this_blocker, Task_token* next_blocker)

s/Archive_member */Archive_member* /


This is OK with those changes.

Thanks.

Ian