From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81048 invoked by alias); 2 May 2017 14:33:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 81037 invoked by uid 89); 2 May 2017 14:33:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=Conversion, 10f, highlighted, HX-Envelope-From:sk:thomas. X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 02 May 2017 14:33:16 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 811BE2B for ; Tue, 2 May 2017 07:33:17 -0700 (PDT) Received: from [10.2.206.52] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 314683F23B for ; Tue, 2 May 2017 07:33:17 -0700 (PDT) Subject: Re: [PATCH] Fix overload resolution involving rvalue references and cv qualifiers. To: gdb-patches@sourceware.org References: <1493328958-8677-1-git-send-email-keiths@redhat.com> From: Thomas Preudhomme Message-ID: <8c82edcb-2354-902c-d411-866471bac341@foss.arm.com> Date: Tue, 02 May 2017 14:33:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1493328958-8677-1-git-send-email-keiths@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00008.txt.bz2 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 > + > + * 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 > Ulrich Weigand > > 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 > + > + * 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 > > * 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(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 (C))" ".* = 42.*" \ > "print value of f1 on Child&& in f2" > >