This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Canonicalize conversion operators
- From: Pedro Alves <palves at redhat dot com>
- To: Keith Seitz <keiths at redhat dot com>, gdb-patches at sourceware dot org
- Date: Wed, 18 Oct 2017 12:19:55 +0100
- Subject: Re: [PATCH] Canonicalize conversion operators
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 875B2154B7C
- References: <1508278336-8655-1-git-send-email-keiths@redhat.com>
On 10/17/2017 11:12 PM, Keith Seitz wrote:
> Consider a conversion operator such as:
>
> operator foo const* const* ();
>
> There are two small parser problems, highlighted by this test:
>
> (gdb) p operator foo const* const*
> There is no field named operatorfoo const* const *
>
> GDB is looking up the symbol "operatorfoo const* const*" -- it is missing a
> space between the keyword "operator" and the type name "foo const* const*".
> Pedro also discovered this problem and has a patch for it pending on his
> cxx-breakpoint-improvements branch.
>
> However, something not address by Pedro's patch is that this input of the
> user-defined type needs to be canonicalized so that different "spellings" of
> the type are recognized:
>
> (gdb) p operator const foo* const *
> There is no field named operator const foo* const *
>
Indeed.
> @@ -1630,7 +1630,13 @@ oper: OPERATOR NEW
>
> c_print_type ($2, NULL, &buf, -1, 0,
> &type_print_raw_options);
> - $$ = operator_stoken (buf.c_str ());
> +
> + /* This also needs canonicalization. */
> + std::string canon
> + = " " + cp_canonicalize_string (buf.c_str ());
> + if (canon.length () == 1)
> + canon = " " + buf.string ();
> + $$ = operator_stoken (canon.c_str ());
The length() == 1 check gave me pause. It's checking that
cp_canonicalize_string returned empty of course, but it
wasn't super clear on a quick skim.
I think you could write it like this, making that part clearer,
and also saving a few string dups and copies:
/* This also needs canonicalization. */
std::string canon
= cp_canonicalize_string (buf.c_str ());
if (canon.empty ())
canon = std::move (buf.string ());
$$ = operator_stoken ((" " + canon).c_str ());
>
> +# Make sure conversion operator names are canonicalized and properly
> +# "spelled."
Period should outside the quote, I think.
Otherwise OK.
Thanks,
Pedro Alves