gdb.dlang/demangle.exp: update expected output for _D8demangle4testFnZv

Message ID 20220113161016.2248240-1-simon.marchi@efficios.com
State New
Headers show
Series
  • gdb.dlang/demangle.exp: update expected output for _D8demangle4testFnZv
Related show

Commit Message

Simon Marchi via Gdb-patches Jan. 13, 2022, 4:10 p.m.
Since commit ce2d3708bc8b ("Synchronize binutils libiberty sources with
gcc version."), I see this failure:

    demangle _D8demangle4testFnZv^M
    demangle.test(typeof(null))^M
    (gdb) FAIL: gdb.dlang/demangle.exp: _D8demangle4testFnZv

The commit imported the commit 0e32a5aa8bc9 ("libiberty: Add support for
D `typeof(*null)' types") from the gcc repository.  That commit includes
an update to libiberty/testsuite/d-demangle-expected, which updates a
test for the exact same mangled name:

     _D8demangle4testFnZv
    -demangle.test(none)
    +demangle.test(typeof(null))

I don't know anything about D, but give that the change was made by Iain
Buclaw, the D language maintainer, I trust him on that.

Fix our test by updating the expected output in the same way.

Note: it's not really useful to have all these D demangling tests in the
GDB testsuite, since there are demangling tests in libiberty.  We should
consider removing them, but we first need to make sure that everything
that is covered in gdb/testsuite/gdb.dlang/demangle.exp is also covered
in libiberty/testsuite/d-demangle-expected.

Change-Id: If2b290ea8367b8e1e0b90b20d4a6e0bee517952d
---
 gdb/testsuite/gdb.dlang/demangle.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.34.1

Comments

Simon Marchi via Gdb-patches Jan. 14, 2022, 7:45 p.m. | #1
Excerpts from Simon Marchi's message of Januar 13, 2022 5:10 pm:
> Since commit ce2d3708bc8b ("Synchronize binutils libiberty sources with

> gcc version."), I see this failure:

> 

>     demangle _D8demangle4testFnZv^M

>     demangle.test(typeof(null))^M

>     (gdb) FAIL: gdb.dlang/demangle.exp: _D8demangle4testFnZv

> 

> The commit imported the commit 0e32a5aa8bc9 ("libiberty: Add support for

> D `typeof(*null)' types") from the gcc repository.  That commit includes

> an update to libiberty/testsuite/d-demangle-expected, which updates a

> test for the exact same mangled name:

> 

>      _D8demangle4testFnZv

>     -demangle.test(none)

>     +demangle.test(typeof(null))

> 

> I don't know anything about D, but give that the change was made by Iain

> Buclaw, the D language maintainer, I trust him on that.

> 

> Fix our test by updating the expected output in the same way.

> 

> Note: it's not really useful to have all these D demangling tests in the

> GDB testsuite, since there are demangling tests in libiberty.  We should

> consider removing them, but we first need to make sure that everything

> that is covered in gdb/testsuite/gdb.dlang/demangle.exp is also covered

> in libiberty/testsuite/d-demangle-expected.

> 


Hi Simon,

To memory, both gdb and libiberty D demangle tests started out as being
identical, but libiberty has been adding more to it as it's been
improved/updated to support the latest language features.  I see no
issues with removing it if it's becoming problematic to maintain both.

Iain.
Simon Marchi Jan. 14, 2022, 8:09 p.m. | #2
On 2022-01-14 2:45 p.m., Iain Buclaw via Gdb-patches wrote:
> Excerpts from Simon Marchi's message of Januar 13, 2022 5:10 pm:

>> Since commit ce2d3708bc8b ("Synchronize binutils libiberty sources with

>> gcc version."), I see this failure:

>>

>>     demangle _D8demangle4testFnZv^M

>>     demangle.test(typeof(null))^M

>>     (gdb) FAIL: gdb.dlang/demangle.exp: _D8demangle4testFnZv

>>

>> The commit imported the commit 0e32a5aa8bc9 ("libiberty: Add support for

>> D `typeof(*null)' types") from the gcc repository.  That commit includes

>> an update to libiberty/testsuite/d-demangle-expected, which updates a

>> test for the exact same mangled name:

>>

>>      _D8demangle4testFnZv

>>     -demangle.test(none)

>>     +demangle.test(typeof(null))

>>

>> I don't know anything about D, but give that the change was made by Iain

>> Buclaw, the D language maintainer, I trust him on that.

>>

>> Fix our test by updating the expected output in the same way.

>>

>> Note: it's not really useful to have all these D demangling tests in the

>> GDB testsuite, since there are demangling tests in libiberty.  We should

>> consider removing them, but we first need to make sure that everything

>> that is covered in gdb/testsuite/gdb.dlang/demangle.exp is also covered

>> in libiberty/testsuite/d-demangle-expected.

>>

> 

> Hi Simon,

> 

> To memory, both gdb and libiberty D demangle tests started out as being

> identical, but libiberty has been adding more to it as it's been

> improved/updated to support the latest language features.  I see no

> issues with removing it if it's becoming problematic to maintain both.

> 

> Iain.

> 


Ok, thanks for the info.  It doesn't seem to break too often, so I am not
in a hurry to remove them, but we'll keep that in mind for the future.  I
pushed the patch in the mean time.

Thanks!

Simon
Tom Tromey Jan. 14, 2022, 8:12 p.m. | #3
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:


Simon> Note: it's not really useful to have all these D demangling tests in the
Simon> GDB testsuite, since there are demangling tests in libiberty.

Funny story, the libiberty demangler test suite actually started as a
copy of gdb's demangler tests, back in ancient times.

I wouldn't mind removing the gdb copies though.  I don't think they add
much value.  If need be I guess we could figure out a way to add unit
testing to libiberty and run that from gdb's selftests.

Tom

Patch

diff --git a/gdb/testsuite/gdb.dlang/demangle.exp b/gdb/testsuite/gdb.dlang/demangle.exp
index ebc2487fd1b..d45437c72cf 100644
--- a/gdb/testsuite/gdb.dlang/demangle.exp
+++ b/gdb/testsuite/gdb.dlang/demangle.exp
@@ -44,7 +44,7 @@  proc test_d_demangling {} {
     test_demangling "_D8demangle4testFkZv" "demangle.test(uint)"
     test_demangling "_D8demangle4testFlZv" "demangle.test(long)"
     test_demangling "_D8demangle4testFmZv" "demangle.test(ulong)"
-    test_demangling "_D8demangle4testFnZv" "demangle.test(none)"
+    test_demangling "_D8demangle4testFnZv" "demangle.test(typeof(null))"
     test_demangling "_D8demangle4testFoZv" "demangle.test(ifloat)"
     test_demangling "_D8demangle4testFpZv" "demangle.test(idouble)"
     test_demangling "_D8demangle4testFqZv" "demangle.test(cfloat)"