change the location_t type, DECL_SOURCE_LINE etc

[Neil Booth]

Per Bothner wrote:-

> I've looked at the code.  I *think* I understand the idea, but
> as they don't seem to be used much, even in cpplib, I may be
> confused.

Sorry, I thought the comments in the header were enough, though
I admit when I looked at them when this thread started it wasn't
entirely obvious (it's over 1 year since I wrote the code).
I could go over them, or you could reword them to clarify what wasn't
clear to you.

The reason they appear to not be being used it that they are only
needed when crossing the boundary from line->(file, line), which doesn't
happen often like I said.  Everything in cpplib uses only logical lines.
At present, translation is only required for diagnostics and when a token
enters the front end.  The latter case is simplified too, because the
front end just remembers the current map, and uses that to convert to a
physical line number without a binary search.  The current map is updated
only when entering / leaving a file, or by #line, through a call-back.
See comment below about efficiency.

This latter use would be unnecessary too if / when the front-ends
started using them; the boundary would pass to the front-end/back-end
interface, and if the back-ends use them, ultimately to asm output.

> One problem is that it isn't obvious what lines numbers are logical,
> and which are physical.  I can figure it by tracing calls in
> cpplib, but a simple typedef would help.  For example:
> 
> typedef unsigned int file_line_t;

Yes, this would be useful.

> I really would like column numbers.  I also think it is very awkward
> when cpplib takes a (file_line_t, column)-pair.  It is much cleaner
> to fold the column number into the file_line_t.

I would like column numbers too.  However, since the rest of the
compiler wasn't supporting it, and getting the rest of the compiler
to use line-maps as they are isn't entirely easy, I kinda welched.

The main problem getting the compiler to use line-maps is global abuse
of reads (and in some cases writes!) to the "lineno" and "filename"
variables.  These all need to go.  Ultimately we could remove the
ad-hoc file stack implemented in toplev.c.

> Suggestion:  To struct line_map add:
> 
>   /* Number of column in a file_line_t which are column numbers. */
>   int column_bits;
> 
> Then SOURCE_LINE becomes:
> #define SOURCE_LINE(MAP, LINE) \
>   (((LINE) + (MAP)->to_line - (MAP)->from_line) >> (MAP)->column_bits)
> and we get:
> #define SOURCE_COLUMN(MAP, LINE) \
>   (((LINE) + (MAP)->to_line - (MAP)->from_line)
>     & ((1 << (MAP)->column_bits) - 1))
> 
> Of course add_line_map would have to take a column_bits parameter,
> which leads to the issue of what to set it to.  I suggest a default
> of 8, for front-ends that generate column-numbers, and 0 for
> front-ends that don't, or when the line-number gets too high.

That's a neat idea.  We could use say 12 bits for column numbers (my
preferred default) initially, and reduce it if / when the line numbers
get too big.  That would require 256K lines, which I'd guess 99.9%
of files get nowhere near.

You'd need code in the populator of the linemaps to recognize crossing
the boundary somehow.  In cpplib, this would be handle_newline() I
think.

Of course, the line maps have no business storing the column themselves;
they just map logical to physical lines.  The columns would be embedded
in the 32-bit logical_line_t used by the front ends.

I'd like to reserve column number zero to mean "no column stored" or
similar; that would be appropriate for at least the C front ends until
they start using the information.

However, I've just realized that this would mean that location
information would not be monotonic increasing across such a
number-of-bits-change boundary.  This may or may not be a problem.

> If a file_line_t is needed in a situation where the current
> physical column number is more than 255, we have the option of
> making another call to add_line_map with a higher column_bits,
> or ignoring the column numbers (set it to 0).  If the current
> line numbes starts getting really high, we might similarly want
> to reset column_bits to 0, so we don't "use up" the file_line_t
> values too quickly.

Yup.  I wrote the above before reading your paragraph properly.

Fancy implementing something like you've described?

> Another problem:  Currently, the struct line_maps is local to a
> cpp_reader.  If it is going to replace location_t, then of course
> it needs to be global.  Perhaps cpp_create_reader could take an
> extra parameter, which is the line_maps set to use?

Of course; this is just an effect of cpplib insisting on reentrancy.
We do something similar for the global identifier hash table.

Note that the line maps are quite efficient; normally the only map being
used is the most recent one, so no binary lookup is even necessary.
They'd be even more efficient if the whole compiler used them; lookups
and quick translations would be even less frequent.

Full binary lookup only tends to be used in diagnostics to refer to
prior locations.

Neil.