diff mbox

Fix missing range information for "%q+D" format code

Message ID 1449263389.8490.60.camel@surprise
State New
Headers show

Commit Message

David Malcolm Dec. 4, 2015, 9:09 p.m. UTC 
On Fri, 2015-12-04 at 15:45 -0500, David Malcolm wrote:

> On Fri, 2015-12-04 at 12:09 +0100, Bernd Schmidt wrote:
> > On 12/03/2015 09:33 PM, David Malcolm wrote:
> > > The attached patch updates the handling of %q+D, simplifying
> > > the implementation, and ensuring that it retains the range
> > > information of the decl, giving:
> > >
> > > diagnostic-ranges-1.c:6:7: warning: unused variable
> ‘redundant’ [-Wunused-variable]
> > >     int redundant;
> > >         ^~~~~~~~~
> > 
> > For most of it I've convinced myself that it looks OK.
> 
> Thanks.
> 
> > >   void
> > > -rich_location::set_range (unsigned int idx, source_range
> src_range,
> > > -                     bool show_caret_p, bool overwrite_loc_p)
> > > +rich_location::set_range (line_maps *set, unsigned int idx,
> > > +                     source_location loc, bool show_caret_p)
> > >   {
> > 
> > Here you need to update the function comment.
> 
> Is the attached suitable?


> diff --git a/libcpp/line-map.c b/libcpp/line-map.c
> index 333e835..0c8f328 100644
> --- a/libcpp/line-map.c
> +++ b/libcpp/line-map.c
> @@ -2064,14 +2064,14 @@ rich_location::add_range (location_range *src_range)
>    m_ranges[m_num_ranges++] = *src_range;
>  }
>  
> -/* Add or overwrite the range given by IDX.  It must either
> -   overwrite an existing range, or add one *exactly* on the end of
> +/* Add or overwrite the location given by IDX.  It must either
> +   overwrite an existing location, or add one *exactly* on the end of
>     the array.
>  
>     This is primarily for use by gcc when implementing diagnostic
>     format decoders e.g. the "+" in the C/C++ frontends, for handling
>     format codes like "%q+D" (which writes the source location of a
> -   tree back into range 0 of the rich_location).
> +   tree back into location 0 of the rich_location).

Sorry; I realize now that the patch makes the final paragraph
inaccurate:
 
>     If SHOW_CARET_P is true, then the range should be rendered with
>     a caret at its starting location.  This

...in that the range/location is actually printed with the caret at its
*caret* location, not its start location.

Updated patch to comment attached, which rewrites things to clarify the
meaning of SHOW_CARET_P.

Comments

Bernd Schmidt Dec. 7, 2015, 10:31 a.m. UTC | #1 
On 12/04/2015 10:09 PM, David Malcolm wrote:
> Updated patch to comment attached, which rewrites things to clarify the
> meaning of SHOW_CARET_P.

I guess this is OK for now. I think I'll go play with the Fortran 
frontend a bit to see what exactly is going on with its use of set_range 
there, I still think that function is a little clunky.


Bernd
diff mbox

Patch

diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 333e835..209d0fb 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -2064,19 +2064,18 @@  rich_location::add_range (location_range *src_range)
   m_ranges[m_num_ranges++] = *src_range;
 }
 
-/* Add or overwrite the range given by IDX.  It must either
-   overwrite an existing range, or add one *exactly* on the end of
-   the array.
-
-   This is primarily for use by gcc when implementing diagnostic
-   format decoders e.g. the "+" in the C/C++ frontends, for handling
-   format codes like "%q+D" (which writes the source location of a
-   tree back into range 0 of the rich_location).
-
-   If SHOW_CARET_P is true, then the range should be rendered with
-   a caret at its starting location.  This
-   is for use by the Fortran frontend, for implementing the
-   "%C" and "%L" format codes.  */
+/* Add or overwrite the location given by IDX, setting its location to LOC,
+   and setting its "should my caret be printed" flag to SHOW_CARET_P.
+
+   It must either overwrite an existing location, or add one *exactly* on
+   the end of the array.
+
+   This is primarily for use by gcc when implementing diagnostic format
+   decoders e.g.
+   - the "+" in the C/C++ frontends, for handling format codes like "%q+D"
+     (which writes the source location of a tree back into location 0 of
+     the rich_location), and
+   - the "%C" and "%L" format codes in the Fortran frontend.  */
 
 void
 rich_location::set_range (line_maps *set, unsigned int idx,