From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 68089 invoked by alias); 19 Feb 2016 20:05:55 -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 68075 invoked by uid 89); 19 Feb 2016 20:05:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.0 required=5.0 tests=BAYES_40,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=ruc, 726, 72,8, 728 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 20:05:51 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 3CD8D6436D; Fri, 19 Feb 2016 20:05:50 +0000 (UTC) Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1JK5neq012846 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 19 Feb 2016 15:05:49 -0500 From: Keith Seitz Subject: Re: [PATCH v2 11/11] [PR gdb/14441] gdb: testsuite: add rvalue reference tests To: Artemiy Volkov References: <1450661481-31178-1-git-send-email-artemiyv@acm.org> <1453229609-20159-1-git-send-email-artemiyv@acm.org> <1453229609-20159-12-git-send-email-artemiyv@acm.org> X-Enigmail-Draft-Status: N1110 Cc: "gdb-patches@sourceware.org ml" Message-ID: <56C7759D.6030907@redhat.com> Date: Fri, 19 Feb 2016 20:05: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-12-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/msg00622.txt.bz2 On 01/19/2016 10:53 AM, Artemiy Volkov wrote: > testsuite/ChangeLog: > > 2016-01-19 Artemiy Volkov > > * gdb.cp/casts.cc (main): Change decltype() function name. Add > rvalue reference type variables. This is better explained: * gdb.cp/casts.cc (decltype): Rename C++11 reserved keyword ... (decl_type): ... to this. All callers updated. You can then omit the "Change decltype" bit in (main). I would consider the above hunk a separate/obvious/trivial patch that you could submit. AFAICT, HEAD is already broken on this. Not necessary, though. [I'm easy!] > * gdb.cp/casts.exp: Compile with -std=c++11. Add rvalue reference > cast tests. You also changed decltype -> decl_type in this file. > * gdb.cp/cpsizeof.cc: Add rvalue reference type variables. > * gdb.cp/cpsizeof.exp: Compile with -std=c++11. Add rvalue > reference sizeof tests. > * gdb.cp/demangle.exp: Add rvalue reference demangle tests. The above occur in proc test_gnu_style_demangling: * gdb.cp/demangle.exp (test_gnu_style_demangling): ... > * gdb.cp/overload.cc (int main): Add a ctor and some methods > with rvalue reference parameters. Changes need to be mentioned explicitly. "(main)" > * gdb.cp/overload.exp: Compile with -std=c++11. Add rvalue > reference overloading tests. > * gdb.cp/ref-params.cc (int f1): New function taking rvalue > reference parameter. > (int f2): Likewise. > (int mf1): Likewise. > (int mf2): Likewise. "int" not necessary. Just list the the new functions: * gdb.cp/ref-params.cc (f1, f2, mf1, mf2): ... > * gdb.cp/ref-params.exp: Compile with -std=c++11. Add rvalue > reference parameter printing tests. > * gdb.cp/ref-types.cc (int main2): Add rvalue reference type > variables. "int main2" -> "(main2)". > * gdb.cp/ref-types.exp: Compile with -std=c++11. Add rvalue > reference type printing tests. > --- > diff --git a/gdb/testsuite/gdb.cp/casts.cc b/gdb/testsuite/gdb.cp/casts.cc > index 43f112f..d15fed1 100644 > --- a/gdb/testsuite/gdb.cp/casts.cc > +++ b/gdb/testsuite/gdb.cp/casts.cc > @@ -1,3 +1,5 @@ > +#include > + > struct A > { > int a; This change is not mentioned in the ChangeLog. > diff --git a/gdb/testsuite/gdb.cp/cpsizeof.cc b/gdb/testsuite/gdb.cp/cpsizeof.cc > index 2dcaea1..6a8de4a 100644 > --- a/gdb/testsuite/gdb.cp/cpsizeof.cc > +++ b/gdb/testsuite/gdb.cp/cpsizeof.cc > @@ -15,6 +15,8 @@ > You should have received a copy of the GNU General Public License > along with this program. If not, see . */ > > +#include > + This isn't mentioned in the ChangeLog. > struct Class > { > int a; > @@ -44,8 +46,10 @@ typedef Enum e12[12]; > #define T(N) \ > N N ## obj; \ > N& N ## _ref = N ## obj; \ > + N&& N ## _rref = std::move(N ## obj); \ > N* N ## p = &(N ## obj); \ > N*& N ## p_ref = N ## p; \ > + N*&& N ## p_rref = std::move(N ## p); \ > int size_ ## N = sizeof (N ## _ref); \ > int size_ ## N ## p = sizeof (N ## p_ref); \ > > diff --git a/gdb/testsuite/gdb.cp/demangle.exp b/gdb/testsuite/gdb.cp/demangle.exp > index 96c90db..90d7be6 100644 > --- a/gdb/testsuite/gdb.cp/demangle.exp > +++ b/gdb/testsuite/gdb.cp/demangle.exp > @@ -511,7 +547,6 @@ proc test_gnu_style_demangling {} { > "operator!=(void *, BDDFunction::VixB const &)" > test_demangling_exact "gnu: __eq__FPvRCQ211BDDFunction4VixB" \ > "operator==(void *, BDDFunction::VixB const &)" > - > test_demangling_exact "gnu: relativeId__CQ36T_phi210T_preserve8FPC_nextRCQ26T_phi210T_preserveRC10Parameters" \ > "T_phi2::T_preserve::FPC_next::relativeId(T_phi2::T_preserve const &, Parameters const &) const" > Superfluous whitespace change? > @@ -535,6 +570,7 @@ proc test_gnu_style_demangling {} { > pass "gnu: __thunk_64__0RL__list__Q29CosNaming20_proxy_NamingContextUlRPt25_CORBA_Unbounded_Sequence1ZQ29CosNaming7BindingRPQ29CosNaming15BindingIterator" > } > } > + > } > > # Likewise. > @@ -1489,6 +1525,7 @@ proc test_hp_style_demangling {} { > test_demangling_exact "hp: elem__6vectorXTiSN67TRdTFPv_i__Fi" "vector::elem(int)" > test_demangling_exact "hp: X__6vectorXTiSN67TdTPvUP5TRs" "vector::X" > > + > # Named constants in template args > > test_demangling_exact "hp: elem__6vectorXTiA3foo__Fi" "vector::elem(int)" Here, too. > diff --git a/gdb/testsuite/gdb.cp/overload.cc b/gdb/testsuite/gdb.cp/overload.cc > index 5c782a4..a977c43 100644 > --- a/gdb/testsuite/gdb.cp/overload.cc > +++ b/gdb/testsuite/gdb.cp/overload.cc > @@ -1,10 +1,12 @@ > #include > +#include This include needs to be mentioned in the ChangeLog. > diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp > index 0cfa638..cfe7f90 100644 > --- a/gdb/testsuite/gdb.cp/overload.exp > +++ b/gdb/testsuite/gdb.cp/overload.exp > @@ -28,7 +28,7 @@ if { [skip_cplus_tests] } { continue } > > standard_testfile .cc > > -if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} { > +if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++ additional_flags="-std=c++11"}]} { > return -1 > } > Line too long. Either assign the compile options to a variable and pass the variable OR add a '\' to split this line. > @@ -53,7 +53,7 @@ gdb_test "up" ".*main.*" "up from marker1" > set re_class "((struct|class) foo \{${ws}public:|struct foo \{)" > set re_fields "int ifoo;${ws}const char ?\\* ?ccpfoo;" > set XX_fields "int ifoo;${ws}char ?\\* ?ccpfoo;" > -set re_ctor "foo\\(int\\);${ws}foo\\(int, (char const|const char) ?\\*\\);${ws}foo\\(foo ?&\\);" > +set re_ctor "foo\\(int\\);${ws}foo\\(int, (char const|const char) ?\\*\\);${ws}foo\\(foo ?&\\);${ws}foo\\(foo ?&?&\\);" > set re_dtor "~foo\\((void|)\\);" > set XX_dtor "~foo\\(int\\);" > set re_methods "void foofunc\\(int\\);" > @@ -72,6 +72,8 @@ set re_methods "${re_methods}${ws}int overload1arg\\(float\\);" > set re_methods "${re_methods}${ws}int overload1arg\\(double\\);" > set re_methods "${re_methods}${ws}int overload1arg\\(int \\*\\);" > set re_methods "${re_methods}${ws}int overload1arg\\(void \\*\\);" > +set re_methods "${re_methods}${ws}int overload1arg\\(foo &\\);" > +set re_methods "${re_methods}${ws}int overload1arg\\(foo &&\\);" > set re_methods "${re_methods}${ws}int overloadfnarg\\((void|)\\);" > set re_methods "${re_methods}${ws}int overloadfnarg\\(int\\);" > set re_methods "${re_methods}${ws}int overloadfnarg\\(int, int ?\\(\\*\\) ?\\(int\\)\\);" I know that you're following suit, but I try to limit my line-lengths to under 80 columns. It's not always possible/feasible, but it can be done. Please use "append" for appending to a string. Let's try to nip this inefficient shell-ism in the bud: append re_methods "${ws}int overload1arg\\(foo &'\)" > diff --git a/gdb/testsuite/gdb.cp/ref-params.cc b/gdb/testsuite/gdb.cp/ref-params.cc > index 0f7e125..efced2e 100644 > --- a/gdb/testsuite/gdb.cp/ref-params.cc > +++ b/gdb/testsuite/gdb.cp/ref-params.cc > @@ -17,6 +17,8 @@ > > /* Author: Paul N. Hilfinger, AdaCore Inc. */ > > +#include > + > struct Parent { > Parent (int id0) : id(id0) { } > int id; This isn't mentioned in the ChangeLog. > @@ -28,7 +30,12 @@ struct Child : public Parent { > > int f1(Parent& R) > { > - return R.id; /* Set breakpoint marker3 here. */ > + return R.id; /* Set breakpoint marker4 here. */ > +} > + > +int f1(Parent&& R) > +{ > + return R.id; /* Set breakpoint marker5 here. */ > } > > int f2(Child& C) > @@ -36,6 +43,11 @@ int f2(Child& C) > return f1(C); /* Set breakpoint marker2 here. */ > } > > +int f2(Child&& C) > +{ > + return f1(std::move(C)); /* Set breakpoint marker3 here. */ > +} > + I don't think there is any requirement that the comments for gdb_get_line_number be in order. I would not change the comment for f1(Parent&) and just make unique comment for f1(Parent&&). > struct OtherParent { > OtherParent (int other_id0) : other_id(other_id0) { } > int other_id; > @@ -50,11 +62,21 @@ int mf1(OtherParent& R) > return R.other_id; > } > > +int mf1(OtherParent&& R) > +{ > + return R.other_id; > +} > + > int mf2(MultiChild& C) > { > return mf1(C); > } > > +int mf2(MultiChild&& C) > +{ > + return mf1(C); > +} > + > int main(void) > { > Child Q(42); We would like test cases to follow the coding standard, too. [See https://sourceware.org/gdb/wiki/Internals%20GDB-Testsuite-Coding-Standards] int mf1 (OtherParent &&C) { ... } and so on. > @@ -62,8 +84,13 @@ int main(void) > > /* Set breakpoint marker1 here. */ > > + f1(Q); > + f1(QR); > + f1(Child(42)); > + > f2(Q); > f2(QR); > + f2(Child(42)); > > MultiChild MQ(53); > MultiChild& MQR = MQ; Likewise here: "f1 (Q)", "f1 (Child (42))", etc. > diff --git a/gdb/testsuite/gdb.cp/ref-types.cc b/gdb/testsuite/gdb.cp/ref-types.cc > index 2c124a9..dafec19 100644 > --- a/gdb/testsuite/gdb.cp/ref-types.cc > +++ b/gdb/testsuite/gdb.cp/ref-types.cc > @@ -15,6 +15,8 @@ > You should have received a copy of the GNU General Public License > along with this program. If not, see . */ > > +#include > + > int main2(void); > > void marker1 (void) Not mentioned in ChangeLog. > @@ -39,6 +41,18 @@ int main(void) > as[2] = 2; > as[3] = 3; > > + short t = -1; > + short *pt; > + short &&rrt = std::move(t); > + pt = &rrt; > + short *&&rrpt = std::move(pt); > + short at[4]; > + at[0] = 0; > + at[1] = 1; > + at[2] = 2; > + at[3] = 3; > + short (&&rrat)[4] = std::move(at); > + > marker1(); > > main2(); This hunk isn't mentioned in the ChangeLog at all. > @@ -66,15 +80,25 @@ int main2(void) > float F; > double D; > char &rC = C; > + char &&rrC = 'A'; > unsigned char &rUC = UC; > + unsigned char &&rrUC = 21; > short &rS = S; > + short &&rrS = -14; > unsigned short &rUS = US; > + unsigned short &&rrUS = 7; > int &rI = I; > + int &&rrI = 102; > unsigned int &rUI = UI; > + unsigned int &&rrUI = 1002; > long &rL = L; > + long &&rrL = -234; > unsigned long &rUL = UL; > + unsigned long &&rrUL = 234; > float &rF = F; > + float &&rrF = 1.25E10; > double &rD = D; > + double &&rrD = -1.375E-123; > C = 'A'; > UC = 21; > S = -14; > diff --git a/gdb/testsuite/gdb.cp/ref-types.exp b/gdb/testsuite/gdb.cp/ref-types.exp > index 3b557f9..5c7c0e5 100644 > --- a/gdb/testsuite/gdb.cp/ref-types.exp > +++ b/gdb/testsuite/gdb.cp/ref-types.exp > @@ -24,7 +24,7 @@ if { [skip_cplus_tests] } { continue } > > standard_testfile .cc > > -if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} { > +if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++ additional_flags="-std=c++11"}]} { > return -1 > } > Line too long. > @@ -127,6 +127,47 @@ gdb_test "print ras\[1\]" ".\[0-9\]* = 1" "print value of ras\[1\]" > gdb_test "print ras\[2\]" ".\[0-9\]* = 2" "print value of ras\[2\]" > gdb_test "print ras\[3\]" ".\[0-9\]* = 3" "print value of ras\[3\]" > > +# rvalue reference tests > + > +gdb_test_multiple "print rrt" "print value of rrt" { > + -re ".\[0-9\]* = \\(short &&\\) @$hex: -1.*$gdb_prompt $" { > + pass "print value of rrt" > + } > + -re ".\[0-9\]* = \\(short int &&\\) @$hex: -1.*$gdb_prompt $" { > + pass "print value of rrt" > + } > + eof { fail "print rrt ($gdb dumped core) (fixme)" ; gdb_start_again ; } > +} > + You can use an atom for the int part and write this test using the simpler gdb_test, i.e., "= \\(short( int)? &&\\)". PS. I know a lot of the test doesn't do it, but you can use the global variable "decimal", defined by dejagnu: set decimal "\[0-9\]+" > +gdb_test_multiple "ptype rrt" "ptype rrt" { > + -re "type = short &&.*$gdb_prompt $" { pass "ptype rrt" } > + -re "type = short int &&.*$gdb_prompt $" { pass "ptype rrt" } > +} > + Likewise. > +gdb_test "print *rrpt" ".\[0-9\]* = -1" "print value of *rrpt" $decimal or just start with "= ...". That's a common usage pattern in the test suite. > + > +# gdb had a bug about dereferencing a pointer type > +# that would lead to wrong results > +# if we try to examine memory at pointer value. > + > +gdb_test "x /hd rrpt" "$hex:\[ \t\]*-1" "examine value at rrpt" > + > +gdb_test_multiple "ptype rrpt" "ptype rrpt" { > + -re "type = short \\*&&.*$gdb_prompt $" { pass "ptype rrpt" } > + -re "type = short int \\*&&.*$gdb_prompt $" { pass "ptype rrpt" } > +} > + "( int)?" and gdb_test > +gdb_test "print rrat\[0\]" ".\[0-9\]* = 0" "print value of rrat\[0\]" > + $decimal or start with "= ..." > +gdb_test_multiple "ptype rrat" "ptype rrat" { > + -re "type = short \\\(&&\\\)\\\[4\\\].*$gdb_prompt $" { pass "ptype rrat" } > + -re "type = short int \\\(&&\\\)\\\[4\\\].*$gdb_prompt $" { pass "ptype rrat" } > +} > + > +gdb_test "print rrat\[1\]" ".\[0-9\]* = 1" "print value of rrat\[1\]" > +gdb_test "print rrat\[2\]" ".\[0-9\]* = 2" "print value of rrat\[2\]" > +gdb_test "print rrat\[3\]" ".\[0-9\]* = 3" "print value of rrat\[3\]" > + $decimal > if ![runto 'f'] then { > perror "couldn't run to f" > @@ -270,3 +311,96 @@ gdb_test "print rD" \ > ".\[0-9\]* = \\(double &\\) @$hex: -1.375e-123.*" \ > "print value of rD" > > +# > +# test rvalue reference types > +# > + > +gdb_test "ptype rrC" "type = char &&" > + > +gdb_test "ptype rrUC" "type = unsigned char &&" > + > +gdb_test_multiple "ptype rrS" "ptype rrS" { > + -re "type = short &&.*$gdb_prompt $" { pass "ptype rrS" } > + -re "type = short int &&.*$gdb_prompt $" { pass "ptype rrS" } > +} > + > +gdb_test_multiple "ptype rrUS" "ptype rrUS" { > + -re "type = unsigned short &&.*$gdb_prompt $" { pass "ptype rrUS" } > + -re "type = short unsigned int &&.*$gdb_prompt $" { pass "ptype rrUS" } > +} > + "( int)?" and gdb_test > +gdb_test "ptype rrI" "type = int &&" > + > +gdb_test "ptype rrUI" "type = unsigned int &&" > + > +gdb_test_multiple "ptype rrL" "ptype rrL" { > + -re "type = long &&.*$gdb_prompt $" { pass "ptype rrL" } > + -re "type = long int &&.*$gdb_prompt $" { pass "ptype rrL" } > +} > + > +gdb_test_multiple "ptype rrUL" "ptype rrUL" { > + -re "type = unsigned long &&.*$gdb_prompt $" { pass "ptype rrUL" } > + -re "type = long unsigned int &&.*$gdb_prompt $" { pass "ptype rrUL" } > +} > + Can use an atom here, too. > +gdb_test "ptype rrF" "type = float &&" > + > +gdb_test "ptype rrD" "type = double &&" > + > +gdb_test "print rrC" ".\[0-9\]* = \\(char &&\\) @$hex: 65 \'A\'" \ > + "print value of rrC" > + > +gdb_test "print rrUC" \ > + ".\[0-9\]* = \\(unsigned char &&\\) @$hex: 21 \'.025\'" \ > + "print value of rrUC" > + > +gdb_test_multiple "print rrS" "print value of rrS" { > + -re ".\[0-9\]* = \\(short &&\\) @$hex: -14.*$gdb_prompt $" { > + pass "print value of rrS" > + } > + -re ".\[0-9\]* = \\(short int &&\\) @$hex: -14.*$gdb_prompt $" { > + pass "print value of rrS" > + } > +} > + > +gdb_test_multiple "print rrUS" "print value of rrUS" { > + -re ".\[0-9\]* = \\(unsigned short &&\\) @$hex: 7.*$gdb_prompt $" { > + pass "print value of rrUS" > + } > + -re ".\[0-9\]* = \\(short unsigned int &&\\) @$hex: 7.*$gdb_prompt $" { > + pass "print value of rrUS" > + } > +} > + > +gdb_test "print rrI" ".\[0-9\]* = \\(int &&\\) @$hex: 102" \ > + "print value of rrI" > + > +gdb_test "print rrUI" \ > + ".\[0-9\]* = \\(unsigned int &&\\) @$hex: 1002" \ > + "print value of UI" > + There are already two tests named "print value of UI," and you've now added a third. Please make the name of this test unique. See https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique for more information. [You need not fix the other tests. Just don't introduce another duplicate test name.] > +gdb_test_multiple "print rrL" "print value of rrL" { > + -re ".\[0-9\]* = \\(long &&\\) @$hex: -234.*$gdb_prompt $" { > + pass "print value of rrL" > + } > + -re ".\[0-9\]* = \\(long int &&\\) @$hex: -234.*$gdb_prompt $" { > + pass "print value of rrL" > + } > +} > + > +gdb_test_multiple "print rrUL" "print value of rrUL" { > + -re ".\[0-9\]* = \\(unsigned long &&\\) @$hex: 234.*$gdb_prompt $" { > + pass "print value of rrUL" > + } > + -re ".\[0-9\]* = \\(long unsigned int &&\\) @$hex: 234.*$gdb_prompt $" { > + pass "print value of rrUL" > + } > +} > + > +gdb_test "print rrF" \ > + ".\[0-9\]* = \\(float &&\\) @$hex: 1.2\[0-9\]*e\\+0?10.*" \ > + "print value of rrF" > + > +gdb_test "print rrD" \ > + ".\[0-9\]* = \\(double &&\\) @$hex: -1.375e-123.*" \ > + "print value of rrD" Atom and/or $decimal in all of the above. IIRC, code was added to python, but I don't see any python tests? Keith