Re: [PATCH v2 11/11] [PR gdb/14441] gdb: testsuite: add rvalue reference tests

[Keith Seitz <keiths at redhat dot com>]

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