On Thu, Jan 23, 2014 at 8:24 PM, Dodji Seketeli <do...@redhat.com> wrote: > Prathamesh Kulkarni <bilbotheelffri...@gmail.com> writes: > >> >> Shall it be correct then to replace calls to error() and friends, >> taking only format string with no-argument specifiers >> to error_at_no_args() ? (similarly we shall need warning_at_no_args, >> pedwarn_no_args, etc.) > > I would guess so, yes. > >>> >>> Also, you'd need to modify cp/error.c:cp_printer in a similar way, to >>> issue an internal_error each time we try to access a null test->args_ptr. >> >> Shall check for text->args_ptr be required in each case label of >> argument specifier in pp_format() >> and client-specific functions like cp_printer() ? > > Yes, I think so. Maybe you can make that a bit more maintainable by > creating a macro like those used to access text->args_ptr in cp_printer, > e.g: > > #define next_int va_arg (*text->args_ptr, int) > > In that macro, make the check for text->args_ptr before accessing it, > and then use that macro to access text->args_ptr through the function. >
I was going through diagnostic.c, it appears that many functions in (error(), error_at(), warning(), etc.) share common code (mostly contains call to diagnostic_set_info() followed by call to report_diagnostic()), which I guess can be re-written in terms of emit_diagnostic(), however since it's variadic that's not possible. I wrote a v_emit_diagnostic() function, that takes same arguments as emit_diagnostic(), with additional va_list * at end (va_list * instead of va_list, so I could pass NULL for error_no_args() and friends). Is it correct to write these other functions (emit_diagnostic(), error(), inform(), etc.) in terms of v_emit_diagnostic() ? > -- > Dodji
Index: diagnostic.c =================================================================== --- diagnostic.c (revision 206867) +++ diagnostic.c (working copy) @@ -884,29 +884,38 @@ diagnostic_append_note (diagnostic_conte va_end (ap); } -bool -emit_diagnostic (diagnostic_t kind, location_t location, int opt, - const char *gmsgid, ...) +static bool +v_emit_diagnostic (diagnostic_t kind, location_t location, int opt, const char *gmsgid, va_list *ap_p) { diagnostic_info diagnostic; - va_list ap; bool ret; - - va_start (ap, gmsgid); + if (kind == DK_PERMERROR) { - diagnostic_set_info (&diagnostic, gmsgid, &ap, location, + diagnostic_set_info (&diagnostic, gmsgid, ap_p, location, permissive_error_kind (global_dc)); diagnostic.option_index = permissive_error_option (global_dc); } else { - diagnostic_set_info (&diagnostic, gmsgid, &ap, location, kind); + diagnostic_set_info (&diagnostic, gmsgid, ap_p, location, kind); if (kind == DK_WARNING || kind == DK_PEDWARN) diagnostic.option_index = opt; } ret = report_diagnostic (&diagnostic); - va_end (ap); + return ret; +} + +bool +emit_diagnostic (diagnostic_t kind, location_t location, int opt, + const char *gmsgid, ...) +{ + va_list ap; + bool ret; + + va_start (ap, gmsgid); + ret = v_emit_diagnostic (kind, location, opt, gmsgid, &ap); + va_end (ap); return ret; } @@ -915,12 +924,10 @@ emit_diagnostic (diagnostic_t kind, loca void inform (location_t location, const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_NOTE); - report_diagnostic (&diagnostic); + v_emit_diagnostic (DK_NOTE, location, 0, gmsgid, &ap); va_end (ap); } @@ -947,15 +954,11 @@ inform_n (location_t location, int n, co bool warning (int opt, const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; bool ret; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_WARNING); - diagnostic.option_index = opt; - - ret = report_diagnostic (&diagnostic); + ret = v_emit_diagnostic (DK_WARNING, input_location, opt, gmsgid, &ap); va_end (ap); return ret; } @@ -967,14 +970,11 @@ warning (int opt, const char *gmsgid, .. bool warning_at (location_t location, int opt, const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; bool ret; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_WARNING); - diagnostic.option_index = opt; - ret = report_diagnostic (&diagnostic); + ret = emit_diagnostic (DK_WARNING, location, opt, gmsgid, &ap); va_end (ap); return ret; } @@ -995,14 +995,11 @@ warning_at (location_t location, int opt bool pedwarn (location_t location, int opt, const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; bool ret; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_PEDWARN); - diagnostic.option_index = opt; - ret = report_diagnostic (&diagnostic); + ret = v_emit_diagnostic (DK_PEDWARN, location, opt, gmsgid, &ap); va_end (ap); return ret; } @@ -1017,15 +1014,11 @@ pedwarn (location_t location, int opt, c bool permerror (location_t location, const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; bool ret; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, location, - permissive_error_kind (global_dc)); - diagnostic.option_index = permissive_error_option (global_dc); - ret = report_diagnostic (&diagnostic); + ret = v_emit_diagnostic (DK_PERMERROR, location, 0, gmsgid, &ap); va_end (ap); return ret; } @@ -1035,12 +1028,10 @@ permerror (location_t location, const ch void error (const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_ERROR); - report_diagnostic (&diagnostic); + v_emit_diagnostic (DK_ERROR, input_location, 0, gmsgid, &ap); va_end (ap); } @@ -1065,12 +1056,10 @@ error_n (location_t location, int n, con void error_at (location_t loc, const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, loc, DK_ERROR); - report_diagnostic (&diagnostic); + v_emit_diagnostic (DK_ERROR, loc, 0, gmsgid, &ap); va_end (ap); } @@ -1080,12 +1069,10 @@ error_at (location_t loc, const char *gm void sorry (const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_SORRY); - report_diagnostic (&diagnostic); + v_emit_diagnostic (DK_SORRY, input_location, 0, gmsgid, &ap); va_end (ap); } @@ -1103,12 +1090,10 @@ seen_error (void) void fatal_error (const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_FATAL); - report_diagnostic (&diagnostic); + v_emit_diagnostic (DK_FATAL, input_location, 0, gmsgid, &ap); va_end (ap); gcc_unreachable (); @@ -1121,12 +1106,10 @@ fatal_error (const char *gmsgid, ...) void internal_error (const char *gmsgid, ...) { - diagnostic_info diagnostic; va_list ap; va_start (ap, gmsgid); - diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_ICE); - report_diagnostic (&diagnostic); + v_emit_diagnostic (DK_ICE, input_location, 0, gmsgid, &ap); va_end (ap); gcc_unreachable ();