[PATCH] fix more -Wformat-diag issues

[Martin Sebor]

On 5/30/19 5:49 PM, Jeff Law wrote:
> On 5/22/19 10:34 AM, Martin Sebor wrote:
>> Incorporating the feedback I got on the -Wformat-diag checker
>> provided an opportunity to tighten up existing and implement
>> a small number of few additional rules based on GCC Coding
>> Conventions (https://gcc.gnu.org/codingconventions.html).
>> The checker now also warns for incorrect uses of the following:
>>
>>   *  bitfield (rather than bit-field)
>>   *  builtin (rather than built-in)
>>   *  command line (rather than command-line)
>>   *  enumeral (rather than enumerated)
>>   *  floating point (rather than floating-point)
>>   *  non-zero (rather than nonzero)
>>
>> In addition, it has become better at finding unquoted qualifiers
>> (like const in const-qualified or "const %qT" rather than %<const
>> %T%>"), and detects some additional abbreviations (e.g., "stmt"
>> instead of "statement").
>>
>> These improvements exposed a number of additional issues in our
>> sources that the attached patch corrects.
>>
>> As before, I have tested the patch on x86_64-linux and adjusted
>> the fallout in the test suite.  More cleanup will likely be needed
>> on other targets but based on the prior changes I don't expect it
>> to be extensive.
>>
>> I will post the patch with the checker implementation separately.
>>
>> Martin
>>
>> PS As discussed in the thread below, some of the instances of
>> added hyphenation in "floating-point" aren't strictly necessary
>> and the  wording might need to be tweaked a bit to make it so:
>>  Â Â https://gcc.gnu.org/ml/gcc/2019-05/msg00168.html
>> I'll handle it in a subsequent commit if it's viewed important.
>>
>> gcc-wformat-diag.diff
>>
>> gcc/c/ChangeLog:
>>
>> 	* c-decl.c (start_decl): Adjust quoting and hyphenation
>> 	in diagnostics.
>> 	(finish_decl): Same.
>> 	(finish_enum): Same.
>> 	(start_function): Same.
>> 	(declspecs_add_type): Same.
>> 	* c-parser.c (warn_for_abs): Same.
>> 	* c-typeck.c (build_binary_op): Same.
>>
>> gcc/c-family/ChangeLog:
>>
>> 	* c-attribs.c (handle_mode_attribute): Adjust quoting and hyphenation.
>> 	(handle_alias_ifunc_attribute): Same.
>> 	(handle_copy_attribute): Same.
>> 	(handle_weakref_attribute): Same.
>> 	(handle_nonnull_attribute): Same.
>> 	* c-warn.c (warn_for_sign_compare): Same.
>> 	(warn_for_restrict): Same.
>> 	* c.opt: Same.
>>
>> 	* c-common.h (GCC_DIAG_STYLE): Adjust.
>> 	 (GCC_DIAG_RAW_STYLE): New macro.
>>
>> gcc/cp/ChangeLog:
>>
>> 	* call.c (build_conditional_expr_1): Adjust quoting and hyphenation.
>> 	(convert_like_real): Same.
>> 	(convert_arg_to_ellipsis): Same.
>> 	* constexpr.c (diag_array_subscript): Same.
>> 	* constraint.cc (diagnose_trait_expression): Same.
>> 	* cvt.c (ocp_convert): Same.
>> 	* decl.c (start_decl): Same.
>> 	(check_for_uninitialized_const_var): Same.
>> 	(grokfndecl): Same.
>> 	(check_special_function_return_type): Same.
>> 	(finish_enum_value_list): Same.
>> 	(start_preparsed_function): Same.
>> 	* parser.c (cp_parser_decl_specifier_seq): Same.
>> 	* typeck.c (cp_build_binary_op): Same.
>> 	(build_static_cast_1): Same.
>>
>> 	* cp-tree.h (GCC_DIAG_STYLE): Adjust.
>> 	 (GCC_DIAG_RAW_STYLE): New macro.
>>
>> gcc/lto/ChangeLog:
>>
>> 	* lto-common.c (lto_file_finalize): Adjust quoting and hyphenation.
>>
>> gcc/objc/ChangeLog:
>>
>> 	* objc-act.c (objc_build_setter_call): Adjust quoting and hyphenation.
>> 	* objc-encoding.c (encode_gnu_bitfield): Same.
>>
>> gcc/ChangeLog:
>>
>> 	* config/i386/i386-features.c (ix86_get_function_versions_dispatcher):
>> 	Adjust quoting and hyphenation.
>> 	* convert.c (convert_to_real_1): Same.
>> 	* gcc.c (driver_wrong_lang_callback): Same.
>> 	(driver::handle_unrecognized_options): Same.
>> 	* gimple-ssa-nonnull-compare.c (do_warn_nonnull_compare): Same.
>> 	* opts-common.c (cmdline_handle_error): Same.
>> 	(read_cmdline_option): Same.
>> 	* opts-global.c (complain_wrong_lang): Same.
>> 	(print_ignored_options): Same.
>> 	(handle_common_deferred_options): Same.
>> 	* pretty-print.h: Same.
>> 	* print-rtl.c (debug_bb_n_slim): Same.
>> 	* sched-rgn.c (make_pass_sched_fusion): Same.
>> 	* tree-cfg.c (verify_gimple_assign_unary): Same.
>> 	(verify_gimple_label): Same.
>> 	* tree-ssa-operands.c (verify_ssa_operands): Same.
>> 	* varasm.c (do_assemble_alias): Same.
>> 	(assemble_alias): Same.
>>
>> 	* diagnostic-core.h (GCC_DIAG_STYLE): Adjust.
>> 	 (GCC_DIAG_RAW_STYLE): New macro.
>>
>> 	* cfghooks.c: Disable -Wformat-diags.
>> 	* cfgloop.c: Same.
>> 	* cfgrtl.c: Same.
>> 	* cgraph.c: Same.
>> 	* diagnostic-show-locus.c: Same.
>> 	* diagnostic.c: Same.
>> 	* gimple-pretty-print.c: Same.
>> 	* graph.c: Same.
>> 	* symtab.c: Same.
>> 	* tree-eh.c Same.
>> 	* tree-pretty-print.c: Same.
>> 	* tree-ssa.c: Same.
>>
>> 	* configure: Regenerate.
>> 	* configure.ac (ACX_PROG_CXX_WARNING_OPTS): Add -Wno-error=format-diag.
>> 	 (ACX_PROG_CC_WARNING_OPTS): Same.
> So in several places there's a comment which indicates that debugging
> dumps and the like do not follow conventions.  Presumably you've tried
> to keep a narrow scope on the diagnostic push/pops.  I'm also concerned
> that the comments you mention that we trigger an ICE.
> 
> So while I'll ack this patch, I would like to know more about the ICE
> that's triggered in the checker and what the plans are for fixing it.

Sorry, I didn't word the comment (copied below) very clearly.
What I meant to say is that the calls to error() in these files
that don't follow the convention are ultimately followed by
an ICE triggered either by an assert (as in cfgloop.c) or a call
to internal_error (cgraph.h).  The diagnostics themselves don't
cause an ICE.

In a comment on one of the i18n bugs raised for these strings
Richard suggests these error calls should probably replaced by
direct calls to the pretty printer.  That would let us avoid
suppressing the warnings and also presumably make it clear to
translators the format strings aren't meant to be translated.
It seemed like too big of a change for this patch so I simply
suppressed the warnings but I agree it's worth considering at
some point.

I'll adjust the comment before I check in the patch (I'm hoping
to commit it at the same time as the checker itself once it's
approved).

+/* Disable warnings about missing quoting in GCC diagnostics for
+   the verification errors.  Their format strings don't follow
+   GCC diagnostic conventions and trigger an ICE in the end.  */
+#if __GNUC__ >= 10
+#  pragma GCC diagnostic push
+#  pragma GCC diagnostic ignored "-Wformat-diag"
+#endif
+

Martin