Re: [PATCH v6 11/11] Add rvalue reference tests and NEWS entry

Pedro Alves
2017-03-13T18:52:15Z

On 03/10/2017 08:04 PM, Keith Seitz wrote:
> This patch adds tests for the initial rvalue reference support patchset. All
> of the new tests are practically mirrored regular references tests and, except
> for the demangler ones, are introduced in new files, which are set to be
> compiled with -std=gnu++11. Tested are printing of rvalue reference types and
> values, rvalue reference parameters in function overloading, demangling of
> function names containing rvalue reference parameters, casts to rvalue
> reference types, application of the sizeof operator to rvalue reference types
> and values, and support for rvalue references within the gdb python module.
>
> Changes since v5:
> o Added NEWS entries
> o Rewrote rvalue-ref-overload.{cc,exp}
> o Updated all tests to current coding standard
> o Added missing copyright headers, maintaining dates from copied files
> o KFAIL failing overload resolution tests

Thanks!

A few nits below. With these addressed, this is good to me.

>
> gdb/ChnageLog
>
> * NEWS: Mention support for ravlue references in GDB and python.

Typo, "ravlue".

> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,6 +1,11 @@
> 2017-MM-DD Keith Seitz <keiths@redhat.com>
>
> PR gdb/14441
> + * NEWS: Mention support for ravlue references in GDB and python.

Again.

> +
> +2017-MM-DD Keith Seitz <keiths@redhat.com>
> +
> + PR gdb/14441
> From Artemiy Volkov <artemiyv@acm.org>
> * gdbtypes.c (rank_one_type): Implement overloading
> resolution rules regarding rvalue references.

> diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-casts.exp b/gdb/testsuite/gdb.cp/rvalue-ref-casts.exp
> new file mode 100644
> index 0000000..7ae4ef0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/rvalue-ref-casts.exp
> @@ -0,0 +1,76 @@
> +# Copyright 2002-2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This file is part of the gdb testsuite
> +
> +# C++11 rvalue reference type casting tests, based on gdb.cp/casts.exp.
> +
> +if {[skip_cplus_tests]} { continue }
> +
> +standard_testfile .cc
> +
> +if {[get_compiler_info "c++"]} {
> + return -1
> +}
> +
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile \
> + {debug c++ additional_flags="-std=gnu++11"}]} {
> + return -1
> +}
> +
> +if {![runto_main]} {
> + untested "couldn't run to main"
> + return -1
> +}
> +
> +# Prevent symbol on address 0x0 being printed.
> +gdb_test_no_output "set print symbol off"
> +
> +set line [gdb_get_line_number {rvalue-ref-casts.exp: 1}]
> +gdb_test "break $line" "Breakpoint.*at.* file .*$srcfile, line $line\\."

Please add an explicit test message in order to avoid it changing
if/when $line changes.

> +
> +gdb_test "continue" "Breakpoint .* at .*$srcfile:$line.*"
> +
> +# Check upcasting.
> +gdb_test "print (A &&) br" ".* = .A &&.* {a = 42}" \
> + "cast derived class rvalue reference to base class rvalue reference"
> +
> +# Check downcasting.
> +gdb_test "print (B &&) ar" ".* = .B.* { = {a = 42}, b = 1729}" \
> + "cast base class rvalue reference to derived class rvalue reference"
> +
> +# Check compiler casting
> +
> +set nonzero_hex "0x\[0-9A-Fa-f\]\[0-9A-Fa-f\]+"
> +
> +gdb_test "print br" ".* = .B.* { = {a = 42}, b = 1729}" \
> + [concat "let compiler cast base class rvalue reference to derived " \
> + "class rvalue reference"]

Is there a special reason for using concat here? I'd think a
continuation would do:

"let compiler cast base class rvalue reference to derived\
class rvalue reference"

TCL replaces continuation+newline+spaces/tabs with a single whitespace:
http://www.tcl.tk/man/tcl8.5/TclCmd/Tcl.htm#M24

> +
> +gdb_test "print static_cast<A &&> (*b)" " = \\(A \\&\\&\\) @$hex: {a = 42}" \
> + "static_cast to rvalue reference type"
> +
> +gdb_test "print reinterpret_cast<A &&> (*b)" \
> + " = \\(A \\&\\&\\) @$hex: {a = 42}" \
> + "reinterpret_cast to rvalue reference type"
> +
> +gdb_test "print dynamic_cast<Alpha &&> (derived)" \
> + [format " = \\(Alpha \\&\\&\\) @%s: {.* = %s( <vtable for Derived.*>)?}" \
> + $nonzero_hex $nonzero_hex] \
> + "dynamic_cast simple upcast to rvalue reference"
> +
> +gdb_test "print dynamic_cast<VirtuallyDerived &&> (*ad)" \
> + "dynamic_cast failed" \
> + "dynamic_cast to rvalue reference to non-existing base"

> new file mode 100644
> index 0000000..7ffb344
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/rvalue-ref-params.exp
> @@ -0,0 +1,64 @@
> +# Copyright 2006-2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Tests for rvalue reference parameters of types and their subtypes in GDB,
> +# based on gdb.cp/ref-params.exp.
> +
> +#
> +# test running programs
> +#
> +
> +if {[skip_cplus_tests]} { continue }
> +
> +standard_testfile .cc
> +
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile \
> + {debug c++ additional_flags="-std=gnu++11"}] == 1} {
> + return -1
> +}
> +
> +proc gdb_start_again {text} {
> + global binfile
> + global srcfile
> +
> + clean_restart $binfile
> +
> + runto ${srcfile}:[gdb_get_line_number $text]

Should probably be wrapped with with_test_prefix to avoid
duplicate messages from within clean_restart / runto,
in case they fail.

> +}
> +
> +gdb_start_again "marker1 here"
> +gdb_test "print f1(static_cast<Child&&>(Q))" ".* = 40.*" \
> + "print value of f1 on (Child&&) in main"
> +
> +gdb_start_again "marker1 here"
> +gdb_test "print f2(static_cast<Child&&>(Q))" ".* = 40.*" \
> + "print value of f2 on (Child&&) in main"
> +
> +gdb_start_again "marker2 here"
> +gdb_test "print C" ".*id = 42.*" "print value of Child&& in f2"
> +
> +setup_kfail "c++/15372" "*-*-*"
> +gdb_test "print f1 (static_cast<Child&&> (C))" ".* = 42.*" \
> + "print value of f1 on Child&& in f2"
> +
> +gdb_start_again "marker3 here"
> +gdb_test "print R" ".*id = 41.*" "print value of Parent&& in f1"
> +
> +gdb_start_again "breakpoint MQ here"
> +gdb_test "print f1(static_cast<MultiChild&&>(MQ))" ".* = 53"
> +gdb_start_again "breakpoint MQ here"
> +gdb_test "print mf1(static_cast<MultiChild&&>(MQ))" ".* = 106"
> +gdb_start_again "breakpoint MQ here"
> +gdb_test "print mf2(static_cast<MultiChild&&>(MQ))" ".* = 106"

> new file mode 100644
> index 0000000..62a665d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/rvalue-ref-types.cc
> @@ -0,0 +1,79 @@
> +/* This test script is part of GDB, the GNU debugger.
> +
> + Copyright 1999-2017 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +/* Tests for reference types with short type variables in GDB, based on
> + gdb.cp/ref-types.cc. */
> +
> +#include <utility>
> +
> +int main2 ();
> +
> +void
> +marker1 ()
> +{
> +}
> +
> +int
> +main ()
> +{
> + 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();
> +
> + return 0;

Indentation of these 3 statements above is odd.

> diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-types.exp b/gdb/testsuite/gdb.cp/rvalue-ref-types.exp
> new file mode 100644
> index 0000000..9067b3b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/rvalue-ref-types.exp
> @@ -0,0 +1,165 @@
> +# Copyright 1999-2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Tests for reference types with short type variables in GDB, based on
> +# gdb.cp/ref-types.exp.
> +

> +#
> +# test running programs
> +#

Eh, we still have that string blindly copied all over
the place. Please don't add another copy. :-)

> +
> +proc gdb_start_again {} {
> + global srcdir
> + global subdir
> + global binfile
> + global gdb_prompt
> + global decimal
> +
> + gdb_start
> + gdb_reinitialize_dir $srcdir/$subdir
> + gdb_load ${binfile}
> +
> + #
> + # set it up at a breakpoint so we can play with the variable values
> + #
> + if {![runto_main]} {
> + perror "couldn't run to breakpoint"
> + continue
> + }
> +
> + if {![runto 'marker1']} {
> + perror "couldn't run to marker1"
> + continue
> + }
> +
> + gdb_test "up" ".*main.*" "up from marker1 2"
> +}

Same comment about with_test_prefix. Actually, can we just
remove the "eof" handling below making this procedure
unnecessary? (I know this is copied from the other file.)

> +
> +
> +gdb_test_multiple "print rrt" "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 ; }
> +}

Thanks,
Pedro Alves