Re: [PATCH] Make all usage of "input_location" be explicit

[David Malcolm <dmalcolm at redhat dot com>]

On Tue, 2013-07-02 at 10:33 -0600, Tom Tromey wrote:
> >>>>> "David" == David Malcolm <dmalcolm@redhat.com> writes:
> 
> David> gcc/java/
> David> 2013-07-02  David Malcolm  <dmalcolm@redhat.com>
> David> 	* class.c (maybe_layout_super_class): Update comment.
> David> 	* decl.c (java_add_stmt): Remove use of input_filename macro.
> David> 	* jcf-parse.c (set_source_filename): Remove use of
> David> 	input_filename macro.
> David> 	(parse_class_file): Remove use of input_line and input_filename
> David> 	macros.
> David> 	(java_parse_file): Remove use of input_filename macro.
> 
> The java parts are ok.  Thanks for doing this.

Thanks; should I wait a while in the hope of getting acceptance of the
full patch (and commit it all as one), or do it dir-by-dir?

> I was curious about this bit:
> 
> David> diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c
> David> index 087cf6a..2942365 100644
> David> --- a/gcc/tree-diagnostic.c
> David> +++ b/gcc/tree-diagnostic.c
> David> @@ -39,7 +39,8 @@ diagnostic_report_current_function (diagnostic_context *context,
> David>  				    diagnostic_info *diagnostic)
> David>  {
> David>    diagnostic_report_current_module (context, diagnostic->location);
> David> -  lang_hooks.print_error_function (context, input_filename, diagnostic);
> David> +  lang_hooks.print_error_function (context, LOCATION_FILE (input_location),
> David> +				   diagnostic);
> 
> I wonder why this needs to use input_location rather than
> diagnostic->location.  (I don't actually know this code, maybe it is
> obvious to those who do.)

I didn't make this change out of conservatism, but I believe it's safe
(not yet tested though).  Here's the result of some archaeology (using
"git blame"):

1999-04-26: In toplev.c, report_error_function began ignoring its "const
char *file" parameter ("as it cannot be relied upon") in r26631 (aka
8f157cdcb53c2138e4cacf4f309b6ff2ac27f257), instead using input_filename,
which at that time was a simple global defined in that file ("char
*input_filename;").

1999-12-18: report_error_function was moved from toplev.c to the then
new diagnostic.c in r31003 (aka
076f8fe18a6ae1eaccafd446bf81351d635f2fae).

2001-06-28: now in diagnostic.c, report_error_function's global
print_error_function callback gained a "diagnostic_context *" param in
r43638 (aka b506dc01ea131f8a9790cc7734fc45c718e9d688), passing it
global_dc.

2002-03-31: the "print_error_function" callback used by
report_error_function was moved into langhooks in r51672 (aka
6c7ff02573fc7155660a0ca60ed96b11963c1271)

2002-06-04:  report_error_function was renamed to
diagnostic_report_current_function in r54291 (aka commit
25e2ffe1cbed24a0896b73bf2ca85571160f4cb1)

2007-09-27: now named diagnostic_report_current_function, the function
gained its 2nd argument (the diagnostic) in r128830 (aka commit
b8c23db329da6afaebde201237f25c583ba61885); the relevant hunk looked like
this:
 void
-diagnostic_report_current_function (diagnostic_context *context)
+diagnostic_report_current_function (diagnostic_context *context,
+                                   diagnostic_info *diagnostic)
 {
   diagnostic_report_current_module (context);
-  lang_hooks.print_error_function (context, input_filename);
+  lang_hooks.print_error_function (context, input_filename,
diagnostic);
 }

2010-05-21: r159685 (aka commit
ce084dfc1cd60d867d38dbed86a914d82fa908d1) moved
diagnostic_report_current_function from diagnostic.c to
tree-diagnostic.c

2011-10-15: now in tree-diagnostic.c, diagnostic_report_current_module
gained its location param in r180083 (aka commit
3de02a0fadcb82dc5ec168ca091a6f47ffe0575b) as part of "Emit macro
expansion related diagnostics":

 diagnostic_report_current_function (diagnostic_context *context,
                                    diagnostic_info *diagnostic)
 {
-  diagnostic_report_current_module (context);
+  diagnostic_report_current_module (context, diagnostic->location);
   lang_hooks.print_error_function (context, input_filename,
diagnostic);
 }

So from my reading of this, that "input_filename" was introduced in
1999-04-26 in r26631 (aka 8f157cdcb53c2138e4cacf4f309b6ff2ac27f257) when
the function was named "report_error_function" and in toplev.c [1]

Echoing Tom's question: given that last 2011 commit for improved macro
diagnostics which appears to assume a diagnostic->location, is it
reasonable to expect every call to diagnostic_report_current_function
call to have the right filename for diagnostic->location for the lang
hook?