From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17037 invoked by alias); 19 Feb 2016 19:46:33 -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 17018 invoked by uid 89); 19 Feb 2016 19:46:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sentences, citation, our, HContent-Transfer-Encoding:8bit X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 19 Feb 2016 19:46:31 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 79CD8C00FD32; Fri, 19 Feb 2016 19:46:30 +0000 (UTC) Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1JJkTjW013127 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 19 Feb 2016 14:46:30 -0500 From: Keith Seitz Subject: Re: [PATCH v2 09/11] [PR gdb/14441] gdb: gdbtypes: add rvalue references to overloading resolution To: Artemiy Volkov References: <1450661481-31178-1-git-send-email-artemiyv@acm.org> <1453229609-20159-1-git-send-email-artemiyv@acm.org> <1453229609-20159-10-git-send-email-artemiyv@acm.org> Cc: gdb-patches@sourceware.org X-Enigmail-Draft-Status: N1110 Message-ID: <56C77115.9090002@redhat.com> Date: Fri, 19 Feb 2016 19:46:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <1453229609-20159-10-git-send-email-artemiyv@acm.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00620.txt.bz2 On 01/19/2016 10:53 AM, Artemiy Volkov wrote: > diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c > index fd17d3c..4bb98c9 100644 > --- a/gdb/gdbtypes.c > +++ b/gdb/gdbtypes.c > @@ -3464,6 +3466,20 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value) > { > struct rank rank = {0,0}; > > + /* Disallow an rvalue argument to bind to a non-const lvalue reference > + parameter and an lvalue argument to bind to an rvalue reference > + parameter. */ Nit: two spaces after all sentences. ["parameter. /*"] > + > + if ((value != NULL) > + && > + ((TYPE_CODE (parm) == TYPE_CODE_REF > + && !TYPE_CONST (parm->main_type->target_type) > + && VALUE_LVAL (value) == not_lval) > + || > + (TYPE_CODE (parm) == TYPE_CODE_RVALUE_REF > + && VALUE_LVAL (value) != not_lval))) > + return INCOMPATIBLE_TYPE_BADNESS; > + I would probably split this up into two statements instead of using "||" to combine, leading each statement with the appropriate comment/citation from the spec. if (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 (TYPE_TARGET_TYPE (parm))) 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; } This is just so much easier to read IMO. For the first test here, is using the target type of the reference sufficient? Do we need to resolve typedefs? In many places, I see the code looping over references (and pointers) to get to the underlying type: for (;;) { type = check_typedef (type); if (TYPE_CODE (type) != TYPE_CODE_PTR && TYPE_CODE (type) != TYPE_CODE_REF) break; type = TYPE_TARGET_TYPE (type); } Maybe you have a test that covers this already (where a reference refers to a typedef'd type)? [If not, please add.] > if (types_equal (parm, arg)) > return EXACT_MATCH_BADNESS; > > @@ -3473,6 +3489,36 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value) > if (TYPE_CODE (arg) == TYPE_CODE_TYPEDEF) > arg = check_typedef (arg); > > + /* 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)); Multi-line statements should be encapsulated in a block. [I know a *lot* of patches have been committed where submitters (and maintainers) haven't followed the rule, but it *is* in our internal coding style wiki. https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Whitespaces ] This appears several times. Keith