This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] fix more -Wformat-diag issues


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]