Re: obj_coff_loc Internal Error

[Tracy Kuhrt <Tracy dot Kuhrt at microchip dot com>]

Nick,

> Hi Tracy,
>
> > > First of all - do you have a binutils copyright assignment on file
> > > with the FSF ?  Without such an assignment we cannot accept your
> > > patch.  (I know that the patch is small, but I consider it to be
> > > non-trivial and not-obvious, so we do need the assignment).
> >
> > I am working on this.  I have yet to receive the form from FSF.
>
> Is this the form you fill out to request copyright assignment ?  I can
> send you a copy directly if you wish.

I actually just received it in the mail.  Now I have to get my employer to
review and sign.

> > > Secondly I am not sure that it is correct to issue a warning if the
> > > comma is missing.  The code before your patch parsed the expression
> > > without requiring an line number and I feel that we should continue to
> > > support that behaviour unless there is a good reason not too.
> >
> > We can remove the warning, but I thought it strange for the
> > directive to accept two arguments that did not require a comma.
>
> True, but it is possible that there are assemblers out there that
> support this syntax.  Certainly this is what is implied by the
> current code.

So if we accept a comma, but don't warn if there is not one that would be
acceptable?

> > > This does look wrong, or at least different.  The listing code does
> > > change the variable 'lineno' which is used by the call to add_lineno()
> > > so the behaviour will change.  Why did you move the call ?  Were you
> > > detecting incorrect line numbers in your output ?
> >
> > I could not find any documentation on this directive except for the
> > comment block before the directive.
> >
> > /* .loc is essentially the same as .ln; parse it for assembler
> >    compatibility.  */
> >
> > I made an assumption that if it were like .ln, then we would want it to
> > perform like .ln.  Maybe "essentially" means that it actually acts
> > differently (?)
>
> It could :-)  Operating on the principle of "if ain't broke, don;t fix
> it" however, I would suggest leaving the current code ordering alone
> until such time as someone can prove that it is wrong.

Gotcha.

> > > Finally, since you are changing the behaviour of the .loc directive, it
> > > would be great if you could add some documentation about it to the
> > > assembler doc file (gas/doc/as.texinfo).  This is optional, but nice.
> >
> > I do not see any documentation for this directive in gas/doc/as.texinfo.
> > I wanted to make sure I understood the directive (and was suggesting the
> > correct patch) before adding any documentation.
>
> Fair enough.  Of course by writing the documentation you also get to
> define the behaviour. :-)  Actually the lack of documentation on this
> and similar directives was why I suggested that you write something in
> the first place.  If you document this directive you, or someone else
> might be motivated to document the others, which in my opinion at
> least, would be a good thing.

I will see what I can do when I get my copyright assignment on file.

Tracy