From: Thomas Preudhomme <thomas.preudhomme@foss.arm.com>
To: gdb-patches@sourceware.org
Subject: Re: [PATCH] Fix overload resolution involving rvalue references and cv qualifiers.
Date: Tue, 02 May 2017 14:33:00 -0000 [thread overview]
Message-ID: <8c82edcb-2354-902c-d411-866471bac341@foss.arm.com> (raw)
In-Reply-To: <1493328958-8677-1-git-send-email-keiths@redhat.com>
Hi Keith,
After your patch I get the following error when running the testsuite for
arm-none-eabi target:
p foo101("abc")^M
evaluation of this expression requires the program to have a function "malloc".^M
(gdb) FAIL: gdb.cp/oranking.exp: p foo101("abc")
I believe the approach in 7b2fe205fd75672d5925fe63f3a0896fa3168aaf will need to
be applied here as well.
Best regards,
Thomas
On 27/04/17 22:35, Keith Seitz wrote:
> The following patch fixes several outstanding overload resolution problems
> with rvalue references and cv qualifiers in the test suite. The tests for
> these problems typically passed with one compiler version and failed with
> another. This behavior occurs because of the ordering of the overloaded
> functions in the debug info. So the first best match "won out" over the
> a subsequent better match.
>
> One of the bugs addressed by this patch is the failure of rank_one_type to
> account for type equality of two overloads based on CV qualifiers. This was
> leading directly to problems evaluating rvalue reference overload quality,
> but it is also highlighted in gdb.cp/oranking.exp, where two test KFAIL as
> a result of this shortcoming.
>
> I found the overload resolution code committed with the rvalue reference
> patch (f9aeb8d49) needlessly over-complicated, and I have greatly simplified
> it. This fixes some KFAILing tests in gdb.exp/rvalue-ref-overload.exp.
>
> Note that there is still one KFAILing test in gdb.cp/rvalue-ref-overload.exp,
> but I hope to address that next.
>
> If there is still time, I would like to push this to the 8.0 branch, too.
>
> I have tested the patch with GCC 5.2.0 and 7.0.0, and both compilers now
> give identical test results.
>
> gdb/ChangeLog
>
> * gdbtypes.c (LVALUE_REFERENCE_TO_RVALUE_BINDING_BADNESS)
> DIFFERENT_REFERENCE_TYPE_BADNESS): Remove.
> (CV_CONVERSION_BADNESS): Define.
> (rank_one_type): Remove overly restrictive rvalue reference
> rank checks.
> Add cv-qualifier checks and subranks for type equality.
> * gdbtypes.h (REFERENCE_CONVERSION_RVALUE,
> REFERENCE_CONVERSION_CONST_LVALUE, CV_CONVERSION_BADNESS,
> CV_CONVERSION_CONST, CV_CONVERSION_VOLATILE): Declare.
>
> gdb/testsuite/ChangeLog
>
> * gdb.cp/oranking.cc (test15): New function.
> (main): Call test15 and declare additional variables for testing.
> * gdb.cp/oranking.exp: Remove kfail status for "p foo4(&a)" and
> "p foo101('abc')" tests.
> * gdb.cp/rvalue-ref-overloads.exp: Remove kfail status for
> "lvalue reference overload" test.
> * gdb.cp/rvalue-ref-params.exp: Remove kfail status for
> "print value of f1 on Child&& in f2" test.
> ---
> gdb/ChangeLog | 12 +++
> gdb/gdbtypes.c | 106 ++++++++++++++-------------
> gdb/gdbtypes.h | 13 +++-
> gdb/testsuite/ChangeLog | 12 +++
> gdb/testsuite/gdb.cp/oranking.cc | 22 ++++++
> gdb/testsuite/gdb.cp/oranking.exp | 9 ++-
> gdb/testsuite/gdb.cp/rvalue-ref-overload.exp | 1 -
> gdb/testsuite/gdb.cp/rvalue-ref-params.exp | 1 -
> 8 files changed, 119 insertions(+), 57 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 8381b8e..7ce2c35 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,15 @@
> +2017-MM-DD Keith Seitz <keiths@redhat.com>
> +
> + * gdbtypes.c (LVALUE_REFERENCE_TO_RVALUE_BINDING_BADNESS)
> + DIFFERENT_REFERENCE_TYPE_BADNESS): Remove.
> + (CV_CONVERSION_BADNESS): Define.
> + (rank_one_type): Remove overly restrictive rvalue reference
> + rank checks.
> + Add cv-qualifier checks and subranks for type equality.
> + * gdbtypes.h (REFERENCE_CONVERSION_RVALUE,
> + REFERENCE_CONVERSION_CONST_LVALUE, CV_CONVERSION_BADNESS,
> + CV_CONVERSION_CONST, CV_CONVERSION_VOLATILE): Declare.
> +
> 2017-04-27 Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
> Ulrich Weigand <uweigand@de.ibm.com>
>
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index dd3992c..c9a9b3d 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -51,6 +51,7 @@ const struct rank EXACT_MATCH_BADNESS = {0,0};
> const struct rank INTEGER_PROMOTION_BADNESS = {1,0};
> const struct rank FLOAT_PROMOTION_BADNESS = {1,0};
> const struct rank BASE_PTR_CONVERSION_BADNESS = {1,0};
> +const struct rank CV_CONVERSION_BADNESS = {1, 0};
> const struct rank INTEGER_CONVERSION_BADNESS = {2,0};
> const struct rank FLOAT_CONVERSION_BADNESS = {2,0};
> const struct rank INT_FLOAT_CONVERSION_BADNESS = {2,0};
> @@ -58,8 +59,6 @@ const struct rank VOID_PTR_CONVERSION_BADNESS = {2,0};
> const struct rank BOOL_CONVERSION_BADNESS = {3,0};
> const struct rank BASE_CONVERSION_BADNESS = {2,0};
> const struct rank REFERENCE_CONVERSION_BADNESS = {2,0};
> -const struct rank LVALUE_REFERENCE_TO_RVALUE_BINDING_BADNESS = {5,0};
> -const struct rank DIFFERENT_REFERENCE_TYPE_BADNESS = {6,0};
> const struct rank NULL_POINTER_CONVERSION_BADNESS = {2,0};
> const struct rank NS_POINTER_CONVERSION_BADNESS = {10,0};
> const struct rank NS_INTEGER_POINTER_CONVERSION_BADNESS = {3,0};
> @@ -3619,57 +3618,51 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
> if (TYPE_CODE (arg) == TYPE_CODE_TYPEDEF)
> arg = check_typedef (arg);
>
> - if (value != NULL)
> + if (TYPE_IS_REFERENCE (parm) && value != NULL)
> {
> - /* An rvalue argument cannot be bound to a non-const lvalue
> - reference parameter... */
> - if (VALUE_LVAL (value) == not_lval
> - && TYPE_CODE (parm) == TYPE_CODE_REF
> - && !TYPE_CONST (parm->main_type->target_type))
> - return INCOMPATIBLE_TYPE_BADNESS;
> -
> - /* ... and an lvalue argument cannot be bound to an rvalue
> - reference parameter. [C++ 13.3.3.1.4p3] */
> - if (VALUE_LVAL (value) != not_lval
> - && TYPE_CODE (parm) == TYPE_CODE_RVALUE_REF)
> - return INCOMPATIBLE_TYPE_BADNESS;
> + if (VALUE_LVAL (value) == not_lval)
> + {
> + /* Rvalues should preferably bind to rvalue references or const
> + lvalue references. */
> + if (TYPE_CODE (parm) == TYPE_CODE_RVALUE_REF)
> + rank.subrank = REFERENCE_CONVERSION_RVALUE;
> + else if (TYPE_CONST (TYPE_TARGET_TYPE (parm)))
> + rank.subrank = REFERENCE_CONVERSION_CONST_LVALUE;
> + else
> + return INCOMPATIBLE_TYPE_BADNESS;
> + return sum_ranks (rank, REFERENCE_CONVERSION_BADNESS);
> + }
> + else
> + {
> + /* Lvalues should prefer lvalue overloads. */
> + if (TYPE_CODE (parm) == TYPE_CODE_RVALUE_REF)
> + {
> + rank.subrank = REFERENCE_CONVERSION_RVALUE;
> + return sum_ranks (rank, REFERENCE_CONVERSION_BADNESS);
> + }
> + }
> }
>
> if (types_equal (parm, arg))
> - return EXACT_MATCH_BADNESS;
> -
> - /* An lvalue reference to a function should get higher priority than an
> - rvalue reference to a function. */
> -
> - if (value != NULL && TYPE_CODE (arg) == TYPE_CODE_RVALUE_REF
> - && TYPE_CODE (TYPE_TARGET_TYPE (arg)) == TYPE_CODE_FUNC)
> - {
> - return (sum_ranks (rank_one_type (parm,
> - lookup_pointer_type (TYPE_TARGET_TYPE (arg)), NULL),
> - DIFFERENT_REFERENCE_TYPE_BADNESS));
> - }
> -
> - /* If a conversion to one type of reference is an identity conversion, and a
> - conversion to the second type of reference is a non-identity conversion,
> - choose the first type. */
> -
> - if (value != NULL && TYPE_IS_REFERENCE (parm) && TYPE_IS_REFERENCE (arg)
> - && TYPE_CODE (parm) != TYPE_CODE (arg))
> {
> - return (sum_ranks (rank_one_type (TYPE_TARGET_TYPE (parm),
> - TYPE_TARGET_TYPE (arg), NULL), DIFFERENT_REFERENCE_TYPE_BADNESS));
> - }
> + struct type *t1 = parm;
> + struct type *t2 = arg;
>
> - /* An rvalue should be first tried to bind to an rvalue reference, and then to
> - an lvalue reference. */
> + /* For pointers and references, compare target type. */
> + if (TYPE_CODE (parm) == TYPE_CODE_PTR || TYPE_IS_REFERENCE (parm))
> + {
> + t1 = TYPE_TARGET_TYPE (parm);
> + t2 = TYPE_TARGET_TYPE (arg);
> + }
>
> - if (value != NULL && TYPE_CODE (parm) == TYPE_CODE_REF
> - && VALUE_LVAL (value) == not_lval)
> - {
> - if (TYPE_IS_REFERENCE (arg))
> - arg = TYPE_TARGET_TYPE (arg);
> - return (sum_ranks (rank_one_type (TYPE_TARGET_TYPE (parm), arg, NULL),
> - LVALUE_REFERENCE_TO_RVALUE_BINDING_BADNESS));
> + /* Make sure they are CV equal, too. */
> + if (TYPE_CONST (t1) != TYPE_CONST (t2))
> + rank.subrank |= CV_CONVERSION_CONST;
> + if (TYPE_VOLATILE (t1) != TYPE_VOLATILE (t2))
> + rank.subrank |= CV_CONVERSION_VOLATILE;
> + if (rank.subrank != 0)
> + return sum_ranks (CV_CONVERSION_BADNESS, rank);
> + return EXACT_MATCH_BADNESS;
> }
>
> /* See through references, since we can almost make non-references
> @@ -3711,10 +3704,23 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
>
> return INCOMPATIBLE_TYPE_BADNESS;
> case TYPE_CODE_ARRAY:
> - if (types_equal (TYPE_TARGET_TYPE (parm),
> - TYPE_TARGET_TYPE (arg)))
> - return EXACT_MATCH_BADNESS;
> - return INCOMPATIBLE_TYPE_BADNESS;
> + {
> + struct type *t1 = TYPE_TARGET_TYPE (parm);
> + struct type *t2 = TYPE_TARGET_TYPE (arg);
> +
> + if (types_equal (t1, t2))
> + {
> + /* Make sure they are CV equal. */
> + if (TYPE_CONST (t1) != TYPE_CONST (t2))
> + rank.subrank |= CV_CONVERSION_CONST;
> + if (TYPE_VOLATILE (t1) != TYPE_VOLATILE (t2))
> + rank.subrank |= CV_CONVERSION_VOLATILE;
> + if (rank.subrank != 0)
> + return sum_ranks (CV_CONVERSION_BADNESS, rank);
> + return EXACT_MATCH_BADNESS;
> + }
> + return INCOMPATIBLE_TYPE_BADNESS;
> + }
> case TYPE_CODE_FUNC:
> return rank_one_type (TYPE_TARGET_TYPE (parm), arg, NULL);
> case TYPE_CODE_INT:
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index f6b4de9..6f896db 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -1891,10 +1891,21 @@ extern const struct rank VOID_PTR_CONVERSION_BADNESS;
> extern const struct rank BOOL_CONVERSION_BADNESS;
> /* * Badness of converting derived to base class. */
> extern const struct rank BASE_CONVERSION_BADNESS;
> -/* * Badness of converting from non-reference to reference. */
> +/* * Badness of converting from non-reference to reference. Subrank
> + is the type of reference conversion being done. */
> extern const struct rank REFERENCE_CONVERSION_BADNESS;
> +/* * Conversion to rvalue reference. */
> +#define REFERENCE_CONVERSION_RVALUE 1
> +/* * Conversion to const lvalue reference. */
> +#define REFERENCE_CONVERSION_CONST_LVALUE 2
> +
> /* * Badness of converting integer 0 to NULL pointer. */
> extern const struct rank NULL_POINTER_CONVERSION;
> +/* * Badness of cv-conversion. Subrank is a flag describing the conversions
> + being done. */
> +extern const struct rank CV_CONVERSION_BADNESS;
> +#define CV_CONVERSION_CONST 1
> +#define CV_CONVERSION_VOLATILE 2
>
> /* Non-standard conversions allowed by the debugger */
>
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index c4d5b79..9909e75 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,15 @@
> +2017-MM-DD Keith Seitz <keiths@redhat.com>
> +
> + * gdb.cp/oranking.cc (test15): New function.
> + (main): Call test15 and declare additional variables for testing.
> + * gdb.cp/oranking.exp: Remove kfail status for "p foo4(&a)" and
> + "p foo101('abc')" tests.
> + Add tests for cv qualifier overloads.
> + * gdb.cp/rvalue-ref-overloads.exp: Remove kfail status for
> + "lvalue reference overload" test.
> + * gdb.cp/rvalue-ref-params.exp: Remove kfail status for
> + "print value of f1 on Child&& in f2" test.
> +
> 2017-04-19 Pedro Alves <palves@redhat.com>
>
> * gdb.threads/threadapply.exp (kill_and_remove_inferior): New
> diff --git a/gdb/testsuite/gdb.cp/oranking.cc b/gdb/testsuite/gdb.cp/oranking.cc
> index dc1972a..bd2f51b 100644
> --- a/gdb/testsuite/gdb.cp/oranking.cc
> +++ b/gdb/testsuite/gdb.cp/oranking.cc
> @@ -147,6 +147,23 @@ int test14 (){
> return foo14(e); // 46
> }
>
> +/* Test cv qualifier overloads. */
> +int foo15 (char *arg) { return 47; }
> +int foo15 (const char *arg) { return 48; }
> +int foo15 (volatile char *arg) { return 49; }
> +int foo15 (const volatile char *arg) { return 50; }
> +static int
> +test15 ()
> +{
> + char *c = 0;
> + const char *cc = 0;
> + volatile char *vc = 0;
> + const volatile char *cvc = 0;
> +
> + // 47 + 48 + 49 + 50 = 194
> + return foo15 (c) + foo15 (cc) + foo15 (vc) + foo15 (cvc);
> +}
> +
> int main() {
> B b;
> foo0(b);
> @@ -203,5 +220,10 @@ int main() {
> foo14(e);
> test14();
>
> + const char *cc = 0;
> + volatile char *vc = 0;
> + const volatile char *cvc = 0;
> + test15 ();
> +
> return 0; // end of main
> }
> diff --git a/gdb/testsuite/gdb.cp/oranking.exp b/gdb/testsuite/gdb.cp/oranking.exp
> index 69efb0c..dc7c78b 100644
> --- a/gdb/testsuite/gdb.cp/oranking.exp
> +++ b/gdb/testsuite/gdb.cp/oranking.exp
> @@ -48,7 +48,6 @@ gdb_test "p test3()" "21"
> gdb_test "p foo3(1.0f)" "21"
>
> gdb_test "p test4()" "24"
> -setup_kfail "gdb/12098" *-*-*
> gdb_test "p foo4(&a)" "24"
>
> gdb_test "p test5()" "26"
> @@ -71,7 +70,6 @@ setup_kfail "gdb/12098" *-*-*
> gdb_test "p foo10(amp)" "216"
>
> gdb_test "p test101()" "218"
> -setup_kfail "gdb/12098" *-*-*
> gdb_test "p foo101(\"abc\")" "218"
>
> gdb_test "p test11()" "32"
> @@ -91,5 +89,8 @@ gdb_test "p test14()" "46"
> setup_kfail "gdb/12096" *-*-*
> gdb_test "p foo14(e)" "46"
>
> -
> -
> +gdb_test "p test15 ()" "194"
> +gdb_test "p foo15 (c)" "47"
> +gdb_test "p foo15 (cc)" "48"
> +gdb_test "p foo15 (vc)" "49"
> +gdb_test "p foo15 (cvc)" "50"
> diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp b/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
> index e729209..5cfd34e 100644
> --- a/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
> +++ b/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
> @@ -60,7 +60,6 @@ gdb_test "print foo_rr_instance1.overload1arg(static_cast<foo&&>(arg))" \
> "print call overloaded func foo && arg"
>
> # Test lvalue vs rvalue function overloads
> -setup_kfail "c++/15372" "*-*-*"
> gdb_test "print f (i)" "= 1" "lvalue reference overload"
>
> gdb_test "print f (ci)" "= 2" "lvalue reference to const overload"
> diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-params.exp b/gdb/testsuite/gdb.cp/rvalue-ref-params.exp
> index 303b447..611b592 100644
> --- a/gdb/testsuite/gdb.cp/rvalue-ref-params.exp
> +++ b/gdb/testsuite/gdb.cp/rvalue-ref-params.exp
> @@ -48,7 +48,6 @@ set t "print value of Child&& in f2"
> gdb_start_again "marker2 here" $t
> gdb_test "print C" ".*id = 42.*" $t
>
> -setup_kfail "c++/15372" "*-*-*"
> gdb_test "print f1 (static_cast<Child&&> (C))" ".* = 42.*" \
> "print value of f1 on Child&& in f2"
>
>
prev parent reply other threads:[~2017-05-02 14:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-27 21:36 Keith Seitz
2017-04-27 21:59 ` Pedro Alves
2017-04-27 23:26 ` Keith Seitz
2017-05-02 14:33 ` Thomas Preudhomme [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8c82edcb-2354-902c-d411-866471bac341@foss.arm.com \
--to=thomas.preudhomme@foss.arm.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox