Re: [libcpp,lto,fortran PATCH] Fix linemap_add use and remove unnecessary kludge
On Sat, Aug 27, 2011 at 9:04 PM, Dodji Seketeli do...@redhat.com wrote: Hello Richard, Richard Guenther richard.guent...@gmail.com writes: In the LTO FE the two linemap_add calls were to advance the location counter to cover the builtin special locations. You exchange these with only one - that doesn't look correct without more explanation. It seems to me that you don't need to worry about advancing the location counter to cover builtin special locations because the lowest possible location that could be handed out by the line map is set to RESERVED_LOCATION_COUNT. You can see that by looking at linemap_init that sets the initial highest location to RESERVED_LOCATION_COUNT - 1, and at linemap_add that sets the starting location of the added map to the highest location + 1. And RESERVED_LOCATION is set to 2 in line-map.h. Hm, ok. That must have changed since the introduction of LTO then. The LTO bits are ok as well. Thanks, Richard. The PCH change is ok. Thank you. -- Dodji
Re: [libcpp,lto,fortran PATCH] Fix linemap_add use and remove unnecessary kludge
Tom Tromey tro...@redhat.com writes: Dodji* line-map.c (linemap_add): Assert that reason must not be DodjiLC_RENAME when called for the first time on a main input file. This is ok. I can't approve the rest but it seems reasonable. Tobias Burnus bur...@net-b.de writes: The Fortran part is OK. Thanks for the patch! Thank you Tom and Tobias. Diego, are the PCH and LTO changes OK? Thanks. -- Dodji
Re: [libcpp,lto,fortran PATCH] Fix linemap_add use and remove unnecessary kludge
On Sat, Aug 27, 2011 at 12:18 PM, Dodji Seketeli do...@redhat.com wrote: Tom Tromey tro...@redhat.com writes: Dodji * line-map.c (linemap_add): Assert that reason must not be Dodji LC_RENAME when called for the first time on a main input file. This is ok. I can't approve the rest but it seems reasonable. Tobias Burnus bur...@net-b.de writes: The Fortran part is OK. Thanks for the patch! Thank you Tom and Tobias. Diego, are the PCH and LTO changes OK? In the LTO FE the two linemap_add calls were to advance the location counter to cover the builtin special locations. You exchange these with only one - that doesn't look correct without more explanation. The PCH change is ok. Thanks, Richard. Thanks. -- Dodji
Re: [libcpp,lto,fortran PATCH] Fix linemap_add use and remove unnecessary kludge
Hello Richard, Richard Guenther richard.guent...@gmail.com writes: In the LTO FE the two linemap_add calls were to advance the location counter to cover the builtin special locations. You exchange these with only one - that doesn't look correct without more explanation. It seems to me that you don't need to worry about advancing the location counter to cover builtin special locations because the lowest possible location that could be handed out by the line map is set to RESERVED_LOCATION_COUNT. You can see that by looking at linemap_init that sets the initial highest location to RESERVED_LOCATION_COUNT - 1, and at linemap_add that sets the starting location of the added map to the highest location + 1. And RESERVED_LOCATION is set to 2 in line-map.h. The PCH change is ok. Thank you. -- Dodji
Re: [libcpp,lto,fortran PATCH] Fix linemap_add use and remove unnecessary kludge
Dodji == Dodji Seketeli do...@redhat.com writes: Dodji * line-map.c (linemap_add): Assert that reason must not be Dodji LC_RENAME when called for the first time on a main input file. This is ok. I can't approve the rest but it seems reasonable. Tom
Re: [libcpp,lto,fortran PATCH] Fix linemap_add use and remove unnecessary kludge
(fortran@ readers: The full patch is at http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01927.html ) On 08/23/2011 08:43 PM, Dodji Seketeli wrote: OK for trunk? The Fortran part is OK. Thanks for the patch! Tobias gcc/fortran/ * scanner.c (load_file): Don't abuse LC_RENAME reason while (indirectly) calling linemap_add. [...] --- a/gcc/fortran/scanner.c +++ b/gcc/fortran/scanner.c @@ -1887,6 +1887,11 @@ load_file (const char *realfilename, const char *displayedname, bool initial) int len, line_len; bool first_line; const char *filename; + /* If realfilename and displayedname are different and non-null then + surely realfilename is the preprocessed form of + displayedname. */ + bool preprocessed_p = (realfilename displayedname + strcmp (realfilename, displayedname)); filename = displayedname ? displayedname : realfilename; @@ -1925,9 +1930,24 @@ load_file (const char *realfilename, const char *displayedname, bool initial) } } - /* Load the file. */ + /* Load the file. - f = get_file (filename, initial ? LC_RENAME : LC_ENTER); + A non-initial file means a file that is being included. In + that case we are creating an LC_ENTER map. + + An initial file means a main file; one that is not included. + That file has already got at least one (surely more) line map(s) + created by gfc_init. So the subsequent map created in that case + must have LC_RENAME reason. + + This latter case is not true for a preprocessed file. In that + case, although the file is initial, the line maps created by + gfc_init was used during the preprocessing of the file. Now that + the preprocessing is over and we are being fed the result of that + preprocessing, we need to create a brand new line map for the + preprocessed file, so the reason is going to be LC_ENTER. */ + + f = get_file (filename, (initial !preprocessed_p) ? LC_RENAME : LC_ENTER); if (!initial) add_file_change (f-filename, f-inclusion_line); current_file = f;
[libcpp,lto,fortran PATCH] Fix linemap_add use and remove unnecessary kludge
Hello, There are a couple of places in the compiler that need to create line maps directly by calling linemap_add. The problem is, sometimes we wrongly do so by passing LC_RENAME as the reason argument even when creating the first map of a given file. But then linemap_add has some code to change that LC_RENAME argument back into LC_ENTER, as it should have been done initially. This patch fixes the few calling spots (in pch, lto and fortran) and turns the then useless fixup kludge in linemap_add into an assert. I'd like this to be applied to trunk as Jason suggested[1] that linemap_add drops that kludge while reviewing my macro location tracking patch set[1]. Bootstrapped with --enable-languages=all,ada --enable-checking and tested on x86_64-unknown-linux-gnu against trunk. OK for trunk? [1]: http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01538.html libcpp/ * line-map.c (linemap_add): Assert that reason must not be LC_RENAME when called for the first time on a main input file. c-family/ * c-pch.c (c_common_read_pch): Call linemap_add with LC_ENTER as it's the first time it's being called on this main TU. gcc/lto/ * lto-lang.c (lto_init): Likewise. Also, avoid calling linemap_add twice. gcc/fortran/ * scanner.c (load_file): Don't abuse LC_RENAME reason while (indirectly) calling linemap_add. --- gcc/c-family/c-pch.c |2 +- gcc/fortran/scanner.c | 24 ++-- gcc/lto/lto-lang.c|3 +-- libcpp/line-map.c |9 - 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/gcc/c-family/c-pch.c b/gcc/c-family/c-pch.c index 3c2fd18..7a289d6 100644 --- a/gcc/c-family/c-pch.c +++ b/gcc/c-family/c-pch.c @@ -446,7 +446,7 @@ c_common_read_pch (cpp_reader *pfile, const char *name, fclose (f); line_table-trace_includes = saved_trace_includes; - linemap_add (line_table, LC_RENAME, 0, saved_loc.file, saved_loc.line); + linemap_add (line_table, LC_ENTER, 0, saved_loc.file, saved_loc.line); /* Give the front end a chance to take action after a PCH file has been loaded. */ diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c index 0c127d4..120d550 100644 --- a/gcc/fortran/scanner.c +++ b/gcc/fortran/scanner.c @@ -1887,6 +1887,11 @@ load_file (const char *realfilename, const char *displayedname, bool initial) int len, line_len; bool first_line; const char *filename; + /* If realfilename and displayedname are different and non-null then + surely realfilename is the preprocessed form of + displayedname. */ + bool preprocessed_p = (realfilename displayedname + strcmp (realfilename, displayedname)); filename = displayedname ? displayedname : realfilename; @@ -1925,9 +1930,24 @@ load_file (const char *realfilename, const char *displayedname, bool initial) } } - /* Load the file. */ + /* Load the file. - f = get_file (filename, initial ? LC_RENAME : LC_ENTER); + A non-initial file means a file that is being included. In + that case we are creating an LC_ENTER map. + + An initial file means a main file; one that is not included. + That file has already got at least one (surely more) line map(s) + created by gfc_init. So the subsequent map created in that case + must have LC_RENAME reason. + + This latter case is not true for a preprocessed file. In that + case, although the file is initial, the line maps created by + gfc_init was used during the preprocessing of the file. Now that + the preprocessing is over and we are being fed the result of that + preprocessing, we need to create a brand new line map for the + preprocessed file, so the reason is going to be LC_ENTER. */ + + f = get_file (filename, (initial !preprocessed_p) ? LC_RENAME : LC_ENTER); if (!initial) add_file_change (f-filename, f-inclusion_line); current_file = f; diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c index 83c41e6..d469fb9 100644 --- a/gcc/lto/lto-lang.c +++ b/gcc/lto/lto-lang.c @@ -1081,8 +1081,7 @@ lto_init (void) flag_generate_lto = flag_wpa; /* Initialize libcpp line maps for gcc_assert to work. */ - linemap_add (line_table, LC_RENAME, 0, NULL, 0); - linemap_add (line_table, LC_RENAME, 0, NULL, 0); + linemap_add (line_table, LC_ENTER, 0, NULL, 0); /* Create the basic integer types. */ build_common_tree_nodes (flag_signed_char, /*short_double=*/false); diff --git a/libcpp/line-map.c b/libcpp/line-map.c index dd3f11c..2a0749a 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -114,11 +114,10 @@ linemap_add (struct line_maps *set, enum lc_reason reason, if (reason == LC_RENAME_VERBATIM) reason = LC_RENAME; - /* If we don't keep our line maps consistent, we can easily - segfault. Don't rely on the client to do it for us. */ - if (set-depth == 0) -reason = LC_ENTER; - else if (reason == LC_LEAVE) + if
