diff mbox

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

Message ID CAESRpQC6p-JYyZKoBXwX+ywRAAidNu9-A3xMvwT82hx73VT0AQ@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez Dec. 4, 2014, 11:36 p.m. UTC 
On 4 December 2014 at 11:21, Tobias Burnus
<tobias.burnus@physik.fu-berlin.de> wrote:
> Manuel López-Ibáñez wrote:
>> It is still not clear to me if line_len is the length of the line read
>> or not, is it? If not, is there any way to actually get the length of
>> the line?
>
> Looking at the code in load_line, the line_len in
>
>    int trunc = load_line (input, &line, &line_len, NULL);
>
> is the size of the buffer. That's not too bad but usually to large. However,
> the actual line length is determined one line later:
>
>    len = gfc_wide_strlen (line);
>
> which does a strlen on gfc_char_t which is uint32_t to accomodate unicode
> characters.
>
>
> Hence, I think
>
>        b->location
> -       = linemap_line_start (line_table, current_file->line++, 120);
> +       = linemap_line_start (line_table, current_file->line++, line_len);
>
> should use "len" instead of "line_len".

Great! That works. It also seems to keep working when I convert all
gfc_error to the common diagnostics (but that still needs a bit more
work).

Dodji, I noticed that there is yet another situation where the offset
computation may fail. If we are in the last map, but no columns have
been allocated, any offset will trigger an assert within
linemap_lookup because the loc+offset > set->highest_location. This is
a bit unfortunate, since being in the last map means that we could
have extended the map or created a new one simply by calling
linemap_position_for_column instead of
linemap_position_for_line_and_column. Do you think that may break
something else?

For now I just added a linemap_assert_fails to fail gracefully if this
ever happens. The patch I committed is below.

Cheers,

Manuel.
diff mbox

Patch

Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog     (revision 218406)
+++ gcc/testsuite/ChangeLog     (revision 218407)
@@ -1,3 +1,7 @@ 
+2014-12-05  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+       * gfortran.dg/line_length_5.f90: New test.
+
 2014-12-04  Sriraman Tallam  <tmsriram@google.com>
            H.J. Lu  <hongjiu.lu@intel.com>

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog       (revision 218406)
+++ gcc/fortran/ChangeLog       (revision 218407)
@@ -1,3 +1,10 @@ 
+2014-12-05  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+       * scanner.c (gfc_next_char_literal): Use gfc_warning_now.
+       (load_file): Use the line length as the column hint for
+       linemap_line_start. Reserve a location for the highest column of
+       the line.
+
 2014-12-03  Manuel López-Ibáñez  <manu@gcc.gnu.org>

        PR fortran/44054
Index: gcc/fortran/scanner.c
===================================================================
--- gcc/fortran/scanner.c       (revision 218406)
+++ gcc/fortran/scanner.c       (revision 218407)
@@ -1056,7 +1056,8 @@ 

          gfc_current_locus.lb->truncated = 0;
          gfc_current_locus.nextc =  gfc_current_locus.lb->line + maxlen;
-         gfc_warning_now_1 ("Line truncated at %L", &gfc_current_locus);
+         gfc_warning_now (OPT_Wline_truncation,
+                          "Line truncated at %L", &gfc_current_locus);
          gfc_current_locus.nextc = current_nextc;
        }

@@ -1195,7 +1196,8 @@ 
          && gfc_current_locus.lb->truncated)
        {
          gfc_current_locus.lb->truncated = 0;
-         gfc_warning_now_1 ("Line truncated at %L", &gfc_current_locus);
+         gfc_warning_now (OPT_Wline_truncation,
+                          "Line truncated at %L", &gfc_current_locus);
        }

       prev_openmp_flag = openmp_flag;
@@ -2044,7 +2046,13 @@ 
                    + (len + 1) * sizeof (gfc_char_t));

       b->location
-       = linemap_line_start (line_table, current_file->line++, 120);
+       = linemap_line_start (line_table, current_file->line++, len);
+      /* ??? We add the location for the maximum column possible here,
+        because otherwise if the next call creates a new line-map, it
+        will not reserve space for any offset.  */
+      if (len > 0)
+       linemap_position_for_column (line_table, len);
+
       b->file = current_file;
       b->truncated = trunc;
       wide_strcpy (b->line, line);
Index: libcpp/line-map.c
===================================================================
--- libcpp/line-map.c   (revision 218406)
+++ libcpp/line-map.c   (revision 218407)
@@ -678,7 +678,8 @@ 
     linemap_position_for_line_and_column (map,
                                          SOURCE_LINE (map, loc),
                                          offset);
-  if (linemap_assert_fails (map == linemap_lookup (set, r)))
+  if (linemap_assert_fails (r <= set->highest_location)
+      || linemap_assert_fails (map == linemap_lookup (set, r)))
     return loc;

   return r;
Index: libcpp/ChangeLog
===================================================================
--- libcpp/ChangeLog    (revision 218406)
+++ libcpp/ChangeLog    (revision 218407)
@@ -1,3 +1,8 @@ 
+2014-12-05  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+       * line-map.c (linemap_position_for_loc_and_offset): Add new
+       linemap_assert_fails.
+
 2014-12-02  Manuel López-Ibáñez  <manu@gcc.gnu.org>

        * include/line-map.h (linemap_assert_fails): Declare.