On Fri, 2016-02-12 at 11:25 -0700, Jeff Law wrote: > On 02/09/2016 01:54 PM, David Malcolm wrote: > > > gcc/ChangeLog: > > PR other/69554 > > * diagnostic-show-locus.c (struct line_span): New struct. > > (layout::get_first_line): Delete. > > (layout::get_last_line): Delete. > > (layout::get_num_line_spans): New member function. > > (layout::get_line_span): Likewise. > > (layout::print_heading_for_line_span_index_p): Likewise. > > (layout::get_expanded_location): Likewise. > > (layout::calculate_line_spans): Likewise. > > (layout::m_first_line): Delete. > > (layout::m_last_line): Delete. > > (layout::m_line_spans): New field. > > (layout::layout): Update comment. Replace m_first_line and > > m_last_line with m_line_spans, replacing their initialization > > with a call to calculate_line_spans. > > (diagnostic_show_locus): When printing source lines and > > annotations, rather than looping over a single span > > of lines, instead loop over each line_span within > > the layout, with an inner loop over the lines within them. > > Call the context's start_span callback when changing line > > spans. > > * diagnostic.c (diagnostic_initialize): Initialize start_span. > > (diagnostic_build_prefix): Break out the building of the > > location > > part of the string into... > > (diagnostic_get_location_text): ...this new function, rewriting > > it from nested ternary expressions to a sequence of "if" > > statements. > > (default_diagnostic_start_span_fn): New function. > > * diagnostic.h (diagnostic_start_span_fn): New typedef. > > (diagnostic_context::start_span): New field. > > (default_diagnostic_start_span_fn): New prototype. > > > > gcc/fortran/ChangeLog: > > PR other/69554 > > * error.c (gfc_diagnostic_start_span): New function. > > (gfc_diagnostics_init): Initialize global_dc's start_span. > > > > gcc/testsuite/ChangeLog: > > PR other/69554 > > * gcc.dg/pr69554-1.c: New test. > > * gfortran.dg/pr69554-1.F90: New test. > > * gfortran.dg/pr69554-2.F90: New test. > > * lib/gcc-dg.exp (proc dg-locus): New function. > > * lib/gfortran-dg.exp (proc gfortran-dg-test): Update comment > > to > > distinguish between the caret-printing and non-caret-printing > > cases. If caret-printing has been explicitly enabled, bail out > > without attempting to fix up the output. > > --- > > gcc/diagnostic-show-locus.c | 226 > > ++++++++++++++++++++++++++++---- > > gcc/diagnostic.c | 62 ++++++--- > > gcc/diagnostic.h | 11 ++ > > gcc/fortran/error.c | 15 +++ > > gcc/testsuite/gcc.dg/pr69554-1.c | 152 > > +++++++++++++++++++++ > > gcc/testsuite/gfortran.dg/pr69554-1.F90 | 28 ++++ > > gcc/testsuite/gfortran.dg/pr69554-2.F90 | 21 +++ > > gcc/testsuite/lib/gcc-dg.exp | 27 ++++ > > gcc/testsuite/lib/gfortran-dg.exp | 19 ++- > > 9 files changed, 520 insertions(+), 41 deletions(-) > > create mode 100644 gcc/testsuite/gcc.dg/pr69554-1.c > > create mode 100644 gcc/testsuite/gfortran.dg/pr69554-1.F90 > > create mode 100644 gcc/testsuite/gfortran.dg/pr69554-2.F90 > > > > diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show > > -locus.c > > index d9b6750..698f42e 100644 > > --- a/gcc/diagnostic-show-locus.c > > +++ b/gcc/diagnostic-show-locus.c > > > @@ -437,8 +477,7 @@ layout::layout (diagnostic_context * context, > > m_colorizer (context, diagnostic), > > m_colorize_source_p (context->colorize_source_p), > > m_layout_ranges (rich_location::MAX_RANGES), > > - m_first_line (m_exploc.line), > > - m_last_line (m_exploc.line), > > + m_line_spans (1 + rich_location::MAX_RANGES), > > m_x_offset (0) > Umm, does that allocate 1 + rich_location::MAX_RANGES linespans? > > > > > + auto_vec<line_span> tmp_spans (1 + rich_location::MAX_RANGES); > > Similarly.
Yes, it's preallocating space in the vecs. rich_location::MAX_RANGES is 3, so there can be at most 4 line spans to consider. A line_span is two linenum_type i.e. a pair of unsigned int, so assuming sizeof(unsigned int) == 4 we have 4 * 2 * 4 = 32 bytes. My thinking here was to preallocate these two vecs with the maximum size they can be, to avoid growing the vec by one each time, thus avoiding "exponential" reallocing and copying. Typically there will be just one or two elements, rather than 4 in this vec, but given the tiny sizes it seemed easiest to preallocate the maximum possible size (granted, N is so small that O(N^2) isn't going to get that big). This pair of allocations is done once per call to diagnostic_show_locus i.e. per diagnostic that ends up printing source code. I believe vl_embed isn't usable here (though I find vec.h to be clear as mud); would a simple array plus length be better? Dave
