diff mbox

[RFAv2,2/4] Fix gdb.ada/rename_subscript_param.exp by using more unique names.

Message ID 20181226140304.19594-3-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers Dec. 26, 2018, 2:03 p.m. UTC 
  The test fails due to conflict between boolean 'b'and some 'b' in atnat.h:

(gdb) print b
Multiple matches for b
[0] cancel
[1] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
[2] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
[3] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> FAIL: gdb.ada/rename_subscript_param.exp: print b before changing its value (timeout)

Fix by renaming 'b' to 'rename_subscript_param_b.
Also, change 'before' to 'after' in the gdb_test message that prints
the value after changing it.
The test however still fails for me, probably because the Debian stable
Ada compiler is too old, and does not properly generate debug info for
this renamining:
(gdb) print rename_subscript_param_b
No definition of "rename_subscript_param_b" in current context.
(gdb) FAIL: gdb.ada/rename_subscript_param.exp: print rename_subscript_param_b before changing its value

Note: if the compiler would generate the correct debug info, the test should
succeed with the name B.  However, waiting for this fix, changing the name
ensures that the test fails directly, instead of causing a timeout.

2018-12-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.ada/rename_subscript_param/pkg.adb (B): Rename to
	Rename_Subscript_Param_B.  All users updated.
---
 gdb/testsuite/gdb.ada/rename_subscript_param.exp     | 8 ++++----
 gdb/testsuite/gdb.ada/rename_subscript_param/pkg.adb | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Joel Brobecker Dec. 26, 2018, 3:27 p.m. UTC | #1 
On Wed, Dec 26, 2018 at 03:03:02PM +0100, Philippe Waroquiers wrote:
> The test fails due to conflict between boolean 'b'and some 'b' in atnat.h:
> 
> (gdb) print b
> Multiple matches for b
> [0] cancel
> [1] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> [2] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> [3] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> > FAIL: gdb.ada/rename_subscript_param.exp: print b before changing its value (timeout)
> 
> Fix by renaming 'b' to 'rename_subscript_param_b.

To me, the way this patch is explained is a little misleading
("the fix"). I would explain instead why you are getting this
multiple-choice question, and why you are electing to rename
the entity...

> Also, change 'before' to 'after' in the gdb_test message that prints
> the value after changing it.
> The test however still fails for me, probably because the Debian stable
> Ada compiler is too old, and does not properly generate debug info for
> this renamining:
> (gdb) print rename_subscript_param_b
> No definition of "rename_subscript_param_b" in current context.
> (gdb) FAIL: gdb.ada/rename_subscript_param.exp: print rename_subscript_param_b before changing its value
> 
> Note: if the compiler would generate the correct debug info, the test should
> succeed with the name B.  However, waiting for this fix, changing the name
> ensures that the test fails directly, instead of causing a timeout.
> 
> 2018-12-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.ada/rename_subscript_param/pkg.adb (B): Rename to
> 	Rename_Subscript_Param_B.  All users updated.

The patch itself looks good, though.
Philippe Waroquiers Dec. 26, 2018, 3:50 p.m. UTC | #2 
On Wed, 2018-12-26 at 19:27 +0400, Joel Brobecker wrote:
> On Wed, Dec 26, 2018 at 03:03:02PM +0100, Philippe Waroquiers wrote:
> > The test fails due to conflict between boolean 'b'and some 'b' in atnat.h:
> > 
> > (gdb) print b
> > Multiple matches for b
> > [0] cancel
> > [1] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> > [2] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> > [3] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> > > FAIL: gdb.ada/rename_subscript_param.exp: print b before changing its value (timeout)
> > 
> > Fix by renaming 'b' to 'rename_subscript_param_b.
> 
> To me, the way this patch is explained is a little misleading
> ("the fix"). I would explain instead why you are getting this
> multiple-choice question, and why you are electing to rename
> the entity...

What about this new commit message ?

--------------------------------------------------
Improve gdb.ada/rename_subscript_param.exp by using more unique names.

With old compilers, the test fails because no debug info is generated
for 'B' and GDB finds some 'b' in atnat.h:

(gdb) print b
Multiple matches for b
[0] cancel
[1] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
[2] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
[3] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> FAIL: gdb.ada/rename_subscript_param.exp: print b before changing its value (timeout)

Avoid the timeout by renaming 'b' to rename_subscript_param_b.

Also, change 'before' to 'after' in the gdb_test message that prints
the value after changing it.

The test still fails with old compilers that do not properly
generate debug info for this renaming:
(gdb) print rename_subscript_param_b
No definition of "rename_subscript_param_b" in current context.
(gdb) FAIL: gdb.ada/rename_subscript_param.exp: print rename_subscript_param_b before changing its value

Note: if the compiler would generate the correct debug info, the test should
succeed with the name B.  However, waiting for this fix, changing the name
ensures that the test fails directly, instead of causing a timeout.

2018-12-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	PR ada/23381
	* gdb.ada/rename_subscript_param/pkg.adb (B): Rename to
	Rename_Subscript_Param_B.  All users updated.
	gdb.ada/rename_subscript_param.exp: Test names made unique.
	Note that PR ada/23381 is only fully fixed when using a recent
	compiler.

> 
> > Also, change 'before' to 'after' in the gdb_test message that prints
> > the value after changing it.
> > The test however still fails for me, probably because the Debian stable
> > Ada compiler is too old, and does not properly generate debug info for
> > this renamining:
> > (gdb) print rename_subscript_param_b
> > No definition of "rename_subscript_param_b" in current context.
> > (gdb) FAIL: gdb.ada/rename_subscript_param.exp: print rename_subscript_param_b before changing its value
> > 
> > Note: if the compiler would generate the correct debug info, the test should
> > succeed with the name B.  However, waiting for this fix, changing the name
> > ensures that the test fails directly, instead of causing a timeout.
> > 
> > 2018-12-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> > 
> > 	* gdb.ada/rename_subscript_param/pkg.adb (B): Rename to
> > 	Rename_Subscript_Param_B.  All users updated.
> 
> The patch itself looks good, though.
>
Joel Brobecker Dec. 27, 2018, 2:35 a.m. UTC | #3 
> What about this new commit message ?

Loks very good. Thanks Philippe!

> 
> --------------------------------------------------
> Improve gdb.ada/rename_subscript_param.exp by using more unique names.
> 
> With old compilers, the test fails because no debug info is generated
> for 'B' and GDB finds some 'b' in atnat.h:
> 
> (gdb) print b
> Multiple matches for b
> [0] cancel
> [1] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> [2] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> [3] b at ../sysdeps/ieee754/dbl-64/atnat.h:106
> > FAIL: gdb.ada/rename_subscript_param.exp: print b before changing its value (timeout)
> 
> Avoid the timeout by renaming 'b' to rename_subscript_param_b.
> 
> Also, change 'before' to 'after' in the gdb_test message that prints
> the value after changing it.
> 
> The test still fails with old compilers that do not properly
> generate debug info for this renaming:
> (gdb) print rename_subscript_param_b
> No definition of "rename_subscript_param_b" in current context.
> (gdb) FAIL: gdb.ada/rename_subscript_param.exp: print rename_subscript_param_b before changing its value
> 
> Note: if the compiler would generate the correct debug info, the test should
> succeed with the name B.  However, waiting for this fix, changing the name
> ensures that the test fails directly, instead of causing a timeout.
> 
> 2018-12-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	PR ada/23381
> 	* gdb.ada/rename_subscript_param/pkg.adb (B): Rename to
> 	Rename_Subscript_Param_B.  All users updated.
> 	gdb.ada/rename_subscript_param.exp: Test names made unique.
> 	Note that PR ada/23381 is only fully fixed when using a recent
> 	compiler.
> 
> > 
> > > Also, change 'before' to 'after' in the gdb_test message that prints
> > > the value after changing it.
> > > The test however still fails for me, probably because the Debian stable
> > > Ada compiler is too old, and does not properly generate debug info for
> > > this renamining:
> > > (gdb) print rename_subscript_param_b
> > > No definition of "rename_subscript_param_b" in current context.
> > > (gdb) FAIL: gdb.ada/rename_subscript_param.exp: print rename_subscript_param_b before changing its value
> > > 
> > > Note: if the compiler would generate the correct debug info, the test should
> > > succeed with the name B.  However, waiting for this fix, changing the name
> > > ensures that the test fails directly, instead of causing a timeout.
> > > 
> > > 2018-12-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> > > 
> > > 	* gdb.ada/rename_subscript_param/pkg.adb (B): Rename to
> > > 	Rename_Subscript_Param_B.  All users updated.
> > 
> > The patch itself looks good, though.
> >
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.ada/rename_subscript_param.exp b/gdb/testsuite/gdb.ada/rename_subscript_param.exp
index 9188bf0fa5..79370cfca2 100644
--- a/gdb/testsuite/gdb.ada/rename_subscript_param.exp
+++ b/gdb/testsuite/gdb.ada/rename_subscript_param.exp
@@ -26,9 +26,9 @@  clean_restart ${testfile}
 set bp_location [gdb_get_line_number "BREAK" ${testdir}/pkg.adb]
 runto "pkg.adb:$bp_location"
 
-gdb_test "print b" \
+gdb_test "print rename_subscript_param_b" \
          " = false" \
-         "print b before changing its value"
+         "print rename_subscript_param_b before changing its value"
 
 set bp_location [gdb_get_line_number "Do_Nothing" ${testdir}/pkg.adb]
 gdb_test "break pkg.adb:$bp_location" \
@@ -39,6 +39,6 @@  gdb_test "cont" \
          "Breakpoint \[0-9\]+, pkg.flip \\(.*" \
          "Continuing to breakpoint on call to Do_Nothing"
 
-gdb_test "print b" \
+gdb_test "print rename_subscript_param_b" \
          " = true" \
-         "print b before changing its value"
+         "print rename_subscript_param_b after changing its value"
diff --git a/gdb/testsuite/gdb.ada/rename_subscript_param/pkg.adb b/gdb/testsuite/gdb.ada/rename_subscript_param/pkg.adb
index cdbcd36ea5..c03fe16ad8 100644
--- a/gdb/testsuite/gdb.ada/rename_subscript_param/pkg.adb
+++ b/gdb/testsuite/gdb.ada/rename_subscript_param/pkg.adb
@@ -21,9 +21,9 @@  package body Pkg is
       --  Create a new scope to check that the scope match algorithm is fine in
       --  the front-end.
       declare
-         B : Boolean renames Bits (I);
+         Rename_Subscript_Param_B : Boolean renames Bits (I);
       begin
-         B := not B; -- BREAK
+         Rename_Subscript_Param_B := not Rename_Subscript_Param_B; -- BREAK
          Do_Nothing (Bits'Address);
       end;
    end Flip;