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: [PATCH] gold: Native Client target support


Roland McGrath <mcgrathr@google.com> writes:

> 1. Do you have a tabs vs spaces policy?
>    The gold code seems to differ from the rest of binutils, where
>    the norm is to use tabs for all leading groups of 8 spaces, as
>    is traditional GNU style.

I intended gold to use the binutils tab settings.  Unfortunately most
Google contributors set their editors to use only spaces, and I decided
to not try to fight that fight.  So gold has a mixture of leading tabs
and all spaces.  This is occasionally mildly irritating but causes
little real harm.


> 2. What's the thinking behind the explicit this-> requirement?
>    I know it's sometimes required in template code for reasons nobody
>    should be required to think about.  Is it just for consistency with
>    those cases, or is it that you want instance variable accesses all to
>    be extremely obviously such (and the trailing_ convention is somehow
>    not sufficient to distinguish them from variables) and all method calls
>    to be obviously distinguished from normal function calls?

I personally find it to be a useful syntactic marker even for member
variables, although I agree that the trailing underscore is sufficient
to avoid all problems.  E.g., adding this-> makes it clear that code
like global = &this->member_; is OK, where global = &member_; looks
fishy at first glance.

Anyhow, that's just style, but for member functions it's important.  In
templated code, omitting the this-> can cause the templated code to call
a function defined outside the class where one would normally expect it
to call a function inside the class.  The rules about which one will be
used are sufficiently confusing that I'm not sure I know them all
myself.  And as you know gold code is very heavily templated, just the
kind of environment where this sort of error might be accidentally
introduced as code changes.


> 3. What about the unnecessary "virtual"?  In, out, or you don't care?
>    (If you don't care, I'll leave them all in, as I find that a useful
>    aid to readability.)

Don't care.


> However, I've discovered that I need to make at least one more substantive
> (though minor) change in the target-selection stuff.  (I did that a while
> after all the rest of the work, and it appears I didn't repeat all the
> testing I'd thought I had.)  I'm adding an off_t argument after Input_file*
> to select_target and Target_selector::{do_,}recognize, which is necessary
> for the Target_selector_nacl logic to apply correctly to files inside
> archives.

Ah, yes.

Ian


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