Skip to content
Snippets Groups Projects
  • Dodji Seketeli's avatar
    c4ca1a09
    Strip "<built-in>" loc from displayed expansion context · c4ca1a09
    Dodji Seketeli authored
    Now that diagnostics for tokens coming from macro expansions point to
    the spelling location of the relevant token (and then displays the
    context of the expansion), some ugly (not so seldom) corner cases can
    happen.
    
    When the relevant token is a built-in token (which means the location
    of that token is BUILTINS_LOCATION) the location prefix displayed to
    the user in the diagnostic line is the "<built-in>:0:0" string.  For
    instance:
    
        <built-in>:0:0: warning: conversion to 'float' alters 'int' constant value
    
    For the user, I think this is surprising and useless.
    
    A more user-friendly approach would be to refer to the first location
    that (in the reported macro expansion context) is for a location in
    real source code, like what is shown in the new test case
    gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C accompanying
    this patch.
    
    To do this, I am making the line-map module provide a new
    linemap_unwind_to_first_non_reserved_loc function that resolves a
    virtual location to the first spelling location that is in real source
    code.
    
    I am then using that facility in the diagnostics printing module and
    in the macro unwinder to avoid printing diagnostics lines that refer
    to the locations for built-ins or more generally for reserved
    locations.  Note that when I start the dance of skipping a built-in
    location I also skip locations that are in system headers, because it
    turned out that a lot of those built-ins are actually used in system
    headers (e.g, "#define INT_MAX __INT_MAX__" where __INT_MAX__ is a
    built-in).
    
    Besides the user-friendliness gain, this patch allows a number of
    regression tests to PASS unchanged with and without
    -ftrack-macro-expansion.
    
    Tested and bootstrapped on x86_64-unknown-linux-gnu against trunk.
    
    Note that the bootstrap with -ftrack-macro-expansion exhibits other
    separate issues that are addressed in subsequent patches.  This patch
    just fixes one class of problems.
    
    The patch does pass bootstrap with -ftrack-macro-expansion turned off,
    though.
    
    libcpp/
    
    	* include/line-map.h (linemap_unwind_toward_expansion): Fix typo
    	in comment.
    	(linemap_unwind_to_first_non_reserved_loc): Declare new function.
    	* line-map.c (linemap_unwind_to_first_non_reserved_loc): Define
    	new function.
    
    gcc/
    
    	* input.c (expand_location_1): When expanding to spelling location
    	in a context of a macro expansion, skip reserved system header
    	locations.  Update comments.  * tree-diagnostic.c
    	(maybe_unwind_expanded_macro_loc): Likewise.
    
    gcc/testsuite/
    
    	* g++.dg/warn/Wconversion-real-integer2.C: New test.
    	* g++.dg/warn/Wconversion-real-integer-3.C: Likewise.
    	* g++.dg/warn/conversion-real-integer-3.h: New header used by the
    	new test above.
    
    From-SVN: r186970
    c4ca1a09
    History
    Strip "<built-in>" loc from displayed expansion context
    Dodji Seketeli authored
    Now that diagnostics for tokens coming from macro expansions point to
    the spelling location of the relevant token (and then displays the
    context of the expansion), some ugly (not so seldom) corner cases can
    happen.
    
    When the relevant token is a built-in token (which means the location
    of that token is BUILTINS_LOCATION) the location prefix displayed to
    the user in the diagnostic line is the "<built-in>:0:0" string.  For
    instance:
    
        <built-in>:0:0: warning: conversion to 'float' alters 'int' constant value
    
    For the user, I think this is surprising and useless.
    
    A more user-friendly approach would be to refer to the first location
    that (in the reported macro expansion context) is for a location in
    real source code, like what is shown in the new test case
    gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C accompanying
    this patch.
    
    To do this, I am making the line-map module provide a new
    linemap_unwind_to_first_non_reserved_loc function that resolves a
    virtual location to the first spelling location that is in real source
    code.
    
    I am then using that facility in the diagnostics printing module and
    in the macro unwinder to avoid printing diagnostics lines that refer
    to the locations for built-ins or more generally for reserved
    locations.  Note that when I start the dance of skipping a built-in
    location I also skip locations that are in system headers, because it
    turned out that a lot of those built-ins are actually used in system
    headers (e.g, "#define INT_MAX __INT_MAX__" where __INT_MAX__ is a
    built-in).
    
    Besides the user-friendliness gain, this patch allows a number of
    regression tests to PASS unchanged with and without
    -ftrack-macro-expansion.
    
    Tested and bootstrapped on x86_64-unknown-linux-gnu against trunk.
    
    Note that the bootstrap with -ftrack-macro-expansion exhibits other
    separate issues that are addressed in subsequent patches.  This patch
    just fixes one class of problems.
    
    The patch does pass bootstrap with -ftrack-macro-expansion turned off,
    though.
    
    libcpp/
    
    	* include/line-map.h (linemap_unwind_toward_expansion): Fix typo
    	in comment.
    	(linemap_unwind_to_first_non_reserved_loc): Declare new function.
    	* line-map.c (linemap_unwind_to_first_non_reserved_loc): Define
    	new function.
    
    gcc/
    
    	* input.c (expand_location_1): When expanding to spelling location
    	in a context of a macro expansion, skip reserved system header
    	locations.  Update comments.  * tree-diagnostic.c
    	(maybe_unwind_expanded_macro_loc): Likewise.
    
    gcc/testsuite/
    
    	* g++.dg/warn/Wconversion-real-integer2.C: New test.
    	* g++.dg/warn/Wconversion-real-integer-3.C: Likewise.
    	* g++.dg/warn/conversion-real-integer-3.h: New header used by the
    	new test above.
    
    From-SVN: r186970