[PATCH fortran/linemap] Add enough column hint to fit any possible offset

[Dodji Seketeli]

Hello Manuel, Tobias,

Manuel López-Ibáñez <lopezibanez@gmail.com> writes:

> This patch actually does not touch linemap but I will appreciate
> Dodji's comments about the approach.

Thanks :-)

> The problem is that in case of long lines, the column hint of 120
> might be too small, thus we do not have enough locations within one
> line to point to a higher column (like 132 in the testcase). Giving as
> column hint the length of the line seems the right fix.

I see.

> The other issue that triggers with this testcase is that, even though
> the column hint allows having line 6 + column 132, since we never
> generate a location except for 6:0, if a new map is created for the
> new line, then it will consider that the last location was 6:0 and the
> new map for line 7 will start at location+1, hence, no offset within
> line 6 can be represented.

Right.  This is done by design to allow for a kind of 'compression' of
the theoretical source location space.  That is, the number of source
locations known to the line map sub-system roughly becomes (roughly) the
number of tokens issued by the tokenizer, rather than the total number
of columns multiplied by the number of lines of the input program.

And of course, we are we today willing something in-between, I guess.
:-)

> My fix here is to create a dummy location for line_len -1 (the last
> column in the line). This forces the next map to start after this last
> column's location.

I think this makes sense.  I'll get to the theoretically unlimited line
length case that Tobias alludes to later below.

> I'm not sure if this latter fix is the approach we want to take.

I think this approach is fine.

> If so, then we may want to change linemap.c itself to force new maps
> to reserve enough locations for any offset in the line instead of
> doing last_location+1.

Hmmh, we must keep in mind that this whole line map thing should keep
working for cases where the primary way to interact with the source
location management sub-system is basically cpp_get_token().  So if that
condition is satisfied, then, why not.  I guess I'd need to look at
specifics of what you are proposing here before talking :-)

[...]

Tobias Burnus <burnus@net-b.de> writes:

[...]

>> My fix here is to create a dummy location for line_len -1 (the last
>> column in the line). This forces the next map to start after this
>> last column's location.
>
> My feeling is that that doesn't work for
> -ffree-line-length-none/-ffixed-line-length-none. In that case,
> gfc_option.free_line_length == 0 and there is no limit to the number
> of characters per line. Thus, I fear that
>
>        b->location
> -	= linemap_line_start (line_table, current_file->line++, 120);
> +	= linemap_line_start (line_table, current_file->line++, line_len);
>
>
> might even set the line length to 0 – but I haven't checked. For all
> other values, it should work.

I am not sure what the value of line_len is when
gfc_option.free_line_length == 0.  If it's not zero, then we are good.
If it's zero (with the semantics of that zero meaning that the line
might be very big), then we might just get the actual length of the line
in terms of column count and pass that to linemap_line_start.

We must just keep in mind that the line subsystem has a hard limit on
column numbers (I think it's 100 000 columns at the moment).  Passed
that limit, we give up tracking column numbers in source locations; that
means that all columns numbers are set to zero.


I hope this is useful.

Cheers,

-- 
		Dodji