On Wed, Apr 29, 2020 at 3:44 PM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi!
>
> The following patch attempts to use the diagnostics URL support if available
> to provide more information about the C++17 empty base and C++20
> [[no_unique_address]] empty class ABI changes in -Wpsabi diagnostics.
>
> GCC 10.1 at the end of the diagnostics is then in some terminals
> underlined with a dotted line and points to a (to be written) anchor in
> gcc-10/changes.html which we need to write anyway.
>
> Ok for trunk if this passes bootstrap/regtest?
>
> 2020-04-29 Jakub Jelinek <ja...@redhat.com>
>
> * configure.ac (-with-changes-root-url): New configure option,
> defaulting to https://gcc.gnu.org/.
> * Makefile.in (CFLAGS-opts.o): Define CHANGES_ROOT_URL for
> opts.c.
> * pretty-print.c (end_url_string): New function.
> (pp_format): Handle %{ and %} for URLs.
> (pp_begin_url): Use pp_string instead of pp_printf.
> (pp_end_url): Use end_url_string.
> * opts.h (get_changes_url): Declare.
> * opts.c (get_changes_url): New function.
> * config/rs6000/rs6000-call.c: Include opts.h.
> (rs6000_discover_homogeneous_aggregate): Use %{GCC 10.1%} instead of
> just GCC 10.1 in diagnostics and add URL.
> * config/arm/arm.c (aapcs_vfp_is_call_or_return_candidate): Likewise.
> * config/aarch64/aarch64.c (aarch64_vfp_is_call_or_return_candidate):
> Likewise.
> * configure: Regenerated.
>
> * c-format.c (PP_FORMAT_CHAR_TABLE): Add %{ and %}.
>
> --- gcc/configure.ac.jj 2020-04-29 10:21:25.061999873 +0200
> +++ gcc/configure.ac 2020-04-29 13:26:51.515516959 +0200
> @@ -986,6 +986,20 @@ AC_ARG_WITH(documentation-root-url,
> )
> AC_SUBST(DOCUMENTATION_ROOT_URL)
>
> +# Allow overriding the default URL for GCC changes
> +AC_ARG_WITH(changes-root-url,
> + AS_HELP_STRING([--with-changes-root-url=URL],
> + [Root for GCC changes URLs]),
> + [case "$withval" in
> + yes) AC_MSG_ERROR([changes root URL not specified]) ;;
> + no) AC_MSG_ERROR([changes root URL not specified]) ;;
> + *) CHANGES_ROOT_URL="$withval"
> + ;;
> + esac],
> + CHANGES_ROOT_URL="https://gcc.gnu.org/";
> +)
> +AC_SUBST(CHANGES_ROOT_URL)
> +
> # Sanity check enable_languages in case someone does not run the toplevel
> # configure # script.
> AC_ARG_ENABLE(languages,
> --- gcc/Makefile.in.jj 2020-02-27 09:28:46.129960135 +0100
> +++ gcc/Makefile.in 2020-04-29 13:38:42.455008718 +0200
> @@ -2187,6 +2187,7 @@ lto-wrapper$(exeext): $(LTO_WRAPPER_OBJS
> mv -f T$@ $@
>
> CFLAGS-opts.o += -DDOCUMENTATION_ROOT_URL=\"@DOCUMENTATION_ROOT_URL@\"
> +CFLAGS-opts.o += -DCHANGES_ROOT_URL=\"@CHANGES_ROOT_URL@\"
>
> # Files used by all variants of C or by the stand-alone pre-processor.
>
> --- gcc/pretty-print.c.jj 2020-04-25 00:08:33.359759328 +0200
> +++ gcc/pretty-print.c 2020-04-29 14:30:46.791792554 +0200
> @@ -1020,6 +1020,8 @@ pp_indent (pretty_printer *pp)
> pp_space (pp);
> }
>
> +static const char *end_url_string (pretty_printer *);
> +
> /* The following format specifiers are recognized as being client
> independent:
> %d, %i: (signed) integer in base ten.
> %u: unsigned integer in base ten.
> @@ -1038,6 +1040,8 @@ pp_indent (pretty_printer *pp)
> %%: '%'.
> %<: opening quote.
> %>: closing quote.
> + %{: URL start.
> + %}: URL end.
> %': apostrophe (should only be used in untranslated messages;
> translations should use appropriate punctuation directly).
> %@: diagnostic_event_id_ptr, for which event_id->known_p () must be true.
> @@ -1051,7 +1055,7 @@ pp_indent (pretty_printer *pp)
> Arguments can be used sequentially, or through %N$ resp. *N$
> notation Nth argument after the format string. If %N$ / *N$
> notation is used, it must be used for all arguments, except %m, %%,
> - %<, %> and %', which may not have a number, as they do not consume
> + %<, %>, %} and %', which may not have a number, as they do not consume
> an argument. When %M$.*N$s is used, M must be N + 1. (This may
> also be written %M$.*s, provided N is not otherwise used.) The
> format string must have conversion specifiers with argument numbers
> @@ -1084,7 +1088,7 @@ pp_format (pretty_printer *pp, text_info
> /* Formatting phase 1: split up TEXT->format_spec into chunks in
> pp_buffer (PP)->args[]. Even-numbered chunks are to be output
> verbatim, odd-numbered chunks are format specifiers.
> - %m, %%, %<, %>, and %' are replaced with the appropriate text at
> + %m, %%, %<, %>, %} and %' are replaced with the appropriate text at
> this point. */
>
> memset (formatters, 0, sizeof formatters);
> @@ -1133,6 +1137,15 @@ pp_format (pretty_printer *pp, text_info
> p++;
> continue;
>
> + case '}':
> + {
> + const char *endurlstr = end_url_string (pp);
> + obstack_grow (&buffer->chunk_obstack, endurlstr,
> + strlen (endurlstr));
> + }
> + p++;
> + continue;
> +
> case 'R':
> {
> const char *colorstr = colorize_stop (pp_show_color (pp));
> @@ -1445,6 +1458,10 @@ pp_format (pretty_printer *pp, text_info
> }
> break;
>
> + case '{':
> + pp_begin_url (pp, va_arg (*text->args_ptr, const char *));
> + break;
> +
> default:
> {
> bool ok;
> @@ -2172,18 +2189,41 @@ void
> pp_begin_url (pretty_printer *pp, const char *url)
> {
> switch (pp->url_format)
> - {
> - case URL_FORMAT_NONE:
> - break;
> - case URL_FORMAT_ST:
> - pp_printf (pp, "\33]8;;%s\33\\", url);
> - break;
> - case URL_FORMAT_BEL:
> - pp_printf (pp, "\33]8;;%s\a", url);
> - break;
> - default:
> - gcc_unreachable ();
> - }
> + {
> + case URL_FORMAT_NONE:
> + break;
> + case URL_FORMAT_ST:
> + pp_string (pp, "\33]8;;");
> + pp_string (pp, url);
> + pp_string (pp, "\33\\");
> + break;
> + case URL_FORMAT_BEL:
> + pp_string (pp, "\33]8;;");
> + pp_string (pp, url);
> + pp_string (pp, "\a");
> + break;
> + default:
> + gcc_unreachable ();
> + }
> +}
> +
> +/* Helper function for pp_end_url and pp_format, return the "close URL"
> escape
> + sequence string. */
> +
> +static const char *
> +end_url_string (pretty_printer *pp)
> +{
> + switch (pp->url_format)
> + {
> + case URL_FORMAT_NONE:
> + return "";
> + case URL_FORMAT_ST:
> + return "\33]8;;\33\\";
> + case URL_FORMAT_BEL:
> + return "\33]8;;\a";
> + default:
> + gcc_unreachable ();
> + }
> }
>
> /* If URL-printing is enabled, write a "close URL" escape sequence to PP. */
> @@ -2191,19 +2231,8 @@ pp_begin_url (pretty_printer *pp, const
> void
> pp_end_url (pretty_printer *pp)
> {
> - switch (pp->url_format)
> - {
> - case URL_FORMAT_NONE:
> - break;
> - case URL_FORMAT_ST:
> - pp_string (pp, "\33]8;;\33\\");
> - break;
> - case URL_FORMAT_BEL:
> - pp_string (pp, "\33]8;;\a");
> - break;
> - default:
> - gcc_unreachable ();
> - }
> + if (pp->url_format != URL_FORMAT_NONE)
> + pp_string (pp, end_url_string (pp));
> }
>
> #if CHECKING_P
> --- gcc/opts.h.jj 2020-02-24 09:02:27.332710905 +0100
> +++ gcc/opts.h 2020-04-29 13:56:55.621826605 +0200
> @@ -464,6 +464,7 @@ extern void parse_options_from_collect_g
> int *);
>
> extern void prepend_xassembler_to_collect_as_options (const char *, obstack
> *);
> +extern char *get_changes_url (const char *);
>
> /* Set OPTION in OPTS to VALUE if the option is not set in OPTS_SET. */
>
> --- gcc/opts.c.jj 2020-04-27 23:28:18.865609469 +0200
> +++ gcc/opts.c 2020-04-29 13:42:13.033891253 +0200
> @@ -3190,6 +3190,15 @@ get_option_url (diagnostic_context *, in
> return NULL;
> }
>
> +/* Given "gcc-10/changes.html#foobar", return that URL under
> + CHANGES_ROOT_URL (see --with-changes-root-url). */
> +
> +char *
> +get_changes_url (const char *str)
> +{
> + return concat (CHANGES_ROOT_URL, str, NULL);
> +}
> +
> #if CHECKING_P
>
> namespace selftest {
> --- gcc/config/arm/arm.c.jj 2020-04-29 13:12:46.736007298 +0200
> +++ gcc/config/arm/arm.c 2020-04-29 14:34:04.878864236 +0200
> @@ -6416,6 +6416,8 @@ aapcs_vfp_is_call_or_return_candidate (e
> && ((alt = aapcs_vfp_sub_candidate (type, &new_mode, NULL))
> != ag_count))
> {
> + const char *url
> + = get_changes_url ("gcc-10/changes.html#empty_base");
This is called even when !warn_psabi_flags and I wonder if we could
instead use a macro at uses, like
> gcc_assert (alt == -1);
> last_reported_type_uid = uid;
> /* Use TYPE_MAIN_VARIANT to strip any redundant const
> @@ -6423,11 +6425,14 @@ aapcs_vfp_is_call_or_return_candidate (e
> if (warn_psabi_flags & WARN_PSABI_NO_UNIQUE_ADDRESS)
> inform (input_location, "parameter passing for argument of "
> "type %qT with %<[[no_unique_address]]%> members "
> - "changed in GCC 10.1", TYPE_MAIN_VARIANT (type));
> + "changed in %{GCC 10.1%}",
> + TYPE_MAIN_VARIANT (type), url);
, CHANGES_URL ("gcc-10/changes.html#empty_base");
where the macro would just use preprocessor string concatenation?
Alternatively 'url' could be static I guess (and never freed)
> else if (warn_psabi_flags & WARN_PSABI_EMPTY_CXX17_BASE)
> inform (input_location, "parameter passing for argument of "
> "type %qT when C++17 is enabled changed to match "
> - "C++14 in GCC 10.1", TYPE_MAIN_VARIANT (type));
> + "C++14 in %{GCC 10.1%}",
> + TYPE_MAIN_VARIANT (type), url);
> + free (url);
> }
> *count = ag_count;
> }
> --- gcc/config/aarch64/aarch64.c.jj 2020-04-29 13:12:46.715007609 +0200
> +++ gcc/config/aarch64/aarch64.c 2020-04-29 14:35:06.712950146 +0200
> @@ -16883,6 +16883,8 @@ aarch64_vfp_is_call_or_return_candidate
> && ((alt = aapcs_vfp_sub_candidate (type, &new_mode, NULL))
> != ag_count))
> {
> + const char *url
> + = get_changes_url ("gcc-10/changes.html#empty_base");
> gcc_assert (alt == -1);
> last_reported_type_uid = uid;
> /* Use TYPE_MAIN_VARIANT to strip any redundant const
> @@ -16890,11 +16892,14 @@ aarch64_vfp_is_call_or_return_candidate
> if (warn_psabi_flags & WARN_PSABI_NO_UNIQUE_ADDRESS)
> inform (input_location, "parameter passing for argument of "
> "type %qT with %<[[no_unique_address]]%> members "
> - "changed in GCC 10.1", TYPE_MAIN_VARIANT (type));
> + "changed in %{GCC 10.1%}",
> + TYPE_MAIN_VARIANT (type), url);
> else if (warn_psabi_flags & WARN_PSABI_EMPTY_CXX17_BASE)
> inform (input_location, "parameter passing for argument of "
> "type %qT when C++17 is enabled changed to match "
> - "C++14 in GCC 10.1", TYPE_MAIN_VARIANT (type));
> + "C++14 in %{GCC 10.1%}",
> + TYPE_MAIN_VARIANT (type), url);
> + free (url);
> }
>
> if (is_ha != NULL) *is_ha = true;
> --- gcc/config/rs6000/rs6000-call.c.jj 2020-04-29 09:00:46.642524337 +0200
> +++ gcc/config/rs6000/rs6000-call.c 2020-04-29 13:50:45.873299042 +0200
> @@ -68,6 +68,7 @@
> #include "tree-vrp.h"
> #include "tree-ssanames.h"
> #include "targhooks.h"
> +#include "opts.h"
>
> #include "rs6000-internal.h"
>
> @@ -5747,16 +5748,18 @@ rs6000_discover_homogeneous_aggregate (m
> unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (type));
> if (uid != last_reported_type_uid)
> {
> + const char *url
> + = get_changes_url ("gcc-10/changes.html#empty_base");
> if (empty_base_seen & 1)
> inform (input_location,
> "parameter passing for argument of type %qT "
> "when C++17 is enabled changed to match C++14
> "
> - "in GCC 10.1", type);
> + "in %{GCC 10.1%}", type, url);
> else
> inform (input_location,
> "parameter passing for argument of type %qT "
> "with %<[[no_unique_address]]%> members "
> - "changed in GCC 10.1", type);
> + "changed in %{GCC 10.1%}", type, url);
> last_reported_type_uid = uid;
> }
> }
> --- gcc/c-family/c-format.c.jj 2020-02-10 15:49:50.821300965 +0100
> +++ gcc/c-family/c-format.c 2020-04-29 13:56:26.926250904 +0200
> @@ -757,6 +757,8 @@ static const format_char_info asm_fprint
> { "<", 0, STD_C89, NOARGUMENTS, "", "<", NULL }, \
> { ">", 0, STD_C89, NOARGUMENTS, "", ">", NULL }, \
> { "'" , 0, STD_C89, NOARGUMENTS, "", "", NULL }, \
> + { "{", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "cR",
> NULL }, \
> + { "}", 0, STD_C89, NOARGUMENTS, "", "", NULL }, \
> { "R", 0, STD_C89, NOARGUMENTS, "", "\\", NULL }, \
> { "m", 0, STD_C89, NOARGUMENTS, "q", "", NULL }, \
> { "Z", 1, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "",
> &gcc_diag_char_table[0] }
> --- gcc/configure.jj 2020-04-29 10:21:25.060999888 +0200
> +++ gcc/configure 2020-04-29 13:28:07.423395043 +0200
> @@ -819,6 +819,7 @@ accel_dir_suffix
> real_target_noncanonical
> enable_as_accelerator
> gnat_install_lib
> +CHANGES_ROOT_URL
> DOCUMENTATION_ROOT_URL
> REPORT_BUGS_TEXI
> REPORT_BUGS_TO
> @@ -967,6 +968,7 @@ with_specs
> with_pkgversion
> with_bugurl
> with_documentation_root_url
> +with_changes_root_url
> enable_languages
> with_multilib_list
> with_zstd
> @@ -1803,6 +1805,8 @@ Optional Packages:
> --with-bugurl=URL Direct users to URL to report a bug
> --with-documentation-root-url=URL
> Root for documentation URLs
> + --with-changes-root-url=URL
> + Root for GCC changes URLs
> --with-multilib-list select multilibs (AArch64, SH and x86-64 only)
> --with-zstd=PATH specify prefix directory for installed zstd
> library.
> Equivalent to --with-zstd-include=PATH/include plus
> @@ -7857,6 +7861,23 @@ fi
>
>
>
> +# Allow overriding the default URL for GCC changes
> +
> +# Check whether --with-changes-root-url was given.
> +if test "${with_changes_root_url+set}" = set; then :
> + withval=$with_changes_root_url; case "$withval" in
> + yes) as_fn_error $? "changes root URL not specified" "$LINENO" 5 ;;
> + no) as_fn_error $? "changes root URL not specified" "$LINENO" 5 ;;
> + *) CHANGES_ROOT_URL="$withval"
> + ;;
> + esac
> +else
> + CHANGES_ROOT_URL="https://gcc.gnu.org/";
> +
> +fi
> +
> +
> +
> # Sanity check enable_languages in case someone does not run the toplevel
> # configure # script.
> # Check whether --enable-languages was given.
> @@ -18988,7 +19009,7 @@ else
> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> lt_status=$lt_dlunknown
> cat > conftest.$ac_ext <<_LT_EOF
> -#line 18991 "configure"
> +#line 19012 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> @@ -19094,7 +19115,7 @@ else
> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> lt_status=$lt_dlunknown
> cat > conftest.$ac_ext <<_LT_EOF
> -#line 19097 "configure"
> +#line 19118 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
>
> Jakub
>
