This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 11/11] [PR gdb/14441] gdb: testsuite: add rvalue reference tests
- From: Keith Seitz <keiths at redhat dot com>
- To: Artemiy Volkov <artemiyv at acm dot org>
- Cc: "gdb-patches at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Date: Fri, 19 Feb 2016 12:05:49 -0800
- Subject: Re: [PATCH v2 11/11] [PR gdb/14441] gdb: testsuite: add rvalue reference tests
- Authentication-results: sourceware.org; auth=none
- References: <1450661481-31178-1-git-send-email-artemiyv at acm dot org> <1453229609-20159-1-git-send-email-artemiyv at acm dot org> <1453229609-20159-12-git-send-email-artemiyv at acm dot org>
On 01/19/2016 10:53 AM, Artemiy Volkov wrote:
> testsuite/ChangeLog:
>
> 2016-01-19 Artemiy Volkov <artemiyv@acm.org>
>
> * 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 <utility>
> +
> 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 <http://www.gnu.org/licenses/>. */
>
> +#include <utility>
> +
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<int,-67,double &,int (void *)>::elem(int)"
> test_demangling_exact "hp: X__6vectorXTiSN67TdTPvUP5TRs" "vector<int,-67,double,void *,5U,short &>::X"
>
> +
> # Named constants in template args
>
> test_demangling_exact "hp: elem__6vectorXTiA3foo__Fi" "vector<int,&foo>::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 <stddef.h>
> +#include <utility>
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 <utility>
> +
> 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 <http://www.gnu.org/licenses/>. */
>
> +#include <utility>
> +
> 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