This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'


On 2018-06-21 14:28, Maciej W. Rozycki wrote:
Hi Simon,

This patch looks good from the GDB side (with one nit in the test below), but somebody from binutils would need to review the bits in opcodes/include.

 Yes, that's why I requested it separately and posted to both mailing
lists.

> +# Verify ABI overrides.
> +mips_disassemble_test bar "move\t\\\$2,\\\$8" "disassemble ABI (numeric)"
> +gdb_test "set disassembler-options"
> +gdb_test "set mips abi o32"
> +mips_disassemble_test bar "move\tv0,t0" "disassemble ABI (o32)"
> +gdb_test "set mips abi n32"
> +mips_disassemble_test bar "move\tv0,a4" "disassemble ABI (n32)"
> +gdb_test "set mips abi n64"
> +mips_disassemble_test bar "move\tv0,a4" "disassemble ABI (n64)"

Avoid parenthesis at the end of test names:

https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

 Thanks for the pointer.  Although it makes sense to me at first glance
that's quite a recent change to a long-established practice. Perhaps it could have been avoided by coding the regression analysis tools referred
more carefully, but I won't be questioning the decision at this point.

That's because of how DejaGNU formats test messages, for example when there is a timeout (as shown in the example on the wiki). We don't have control over that, and we don't want "foo" and "foo (timeout)" to be considered as two different tests.

 The wiki does not indicate a suggested replacement however and I would
rather avoid creating a mess where individual tests would use different
approaches.  Offhand I'd be inclined to use brackets, either square or
angled.  What has been the new practice then?

Often, parenthesis would be used when the same series of tests are ran with different settings. For example:

  foo (non-stop)
  bar (non-stop)
  foo (all-stop)
  bar (all-stop)

For this it's useful to use prefixes, with with_test_prefix, for example.

with_test_prefix "non-stop" {
  ...
}

with_test_prefix "all-stop" {
  ...
}

Or even

foreach_with_prefix mode {all-stop non-stop} {
  ...
}


That does not really apply to your case though. I think here you can just remove the parenthesis, and maybe add a comma.

  mips_disassemble_test bar "move\tv0,a4" "disassemble ABI, n64"

As long as it's clear.

Simon


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]