Re: [RFA] Sort overload menu (decode_line_2)

[Joel Brobecker <brobecker at adacore dot com>]

> While working on my linespec rewrite, I've noticed the (implicit)
> assumption that decode_line_2 outputs the overload menu in code
> order. While this assumption has worked for quite some time, this
> causes problems for the linespec rewrite, which could alter this
> implicit ordering.

That's not the only ordering issue you might get, and I confirm
they can be a real pain! I've had to write some pretty scary regexps
to be able to match orders that flip-flop depending of debugger
version, linker mood and phase of the moon. I wish your patch took
care of that too (not an official request, just commenting on your
patch).

> ChangeLog
> 2012-02-28  Keith Seitz  <keiths@redhat.com>
> 
> 	* linespec.c (decode_line_2): Sort the list of methods
> 	alphabetically before presenting the user with a selection
> 	menu.

FWIW, I personally have no problem with that. But I think it should
be reviewed by someone a little more familiar with C++.

> testsuite/ChangeLog
> 2012-02-28  Keith Seitz  <keiths@redhat.com>
> 
> 	* gdb.cp/method2.exp: Output of overload menu is now
> 	alphabetized.  Update tests for "break A::method".
> 	Use prepare_for_testing.
> 	* gdb.cp/ovldbreak.exp: Use prepare_for_testing.
> 	Update syntax of arcane "if" statement.
> 	Use gdb_get_line_number instead of hard-coding them.
> 	Overload menu is alphabetized: rewrite to accommodate.
> 	Unset variables LINE and TYPES which are used in other tests.
> 	Similarly, compute the output of "info break".
> 	Update the breakpoint table after all breakpoints are deleted.
> 	(continue_to_bp_overloaded): Rename ACTUALS to ARGUMENT and
> 	compute ACTUALS and the method body based on parameters.
> 	Update expected output accordingly.
> 	* gdb.cp/ovldbreak.exp (foo::overload1arg): Reformat and add
                           ^^^ exp -> cc
> 	unique comments to allow the use of gdb_get_line_number.

It would have been cool if you had kept the little cleanups as
a separate patch ("git add -p" is your friend). But that's better
than not doing the cleanup at all, so I won't complain :-).

Just a question, wondering if that matters at all. Do we have a
convention for indentation in TCL/expect files? I am sort of used
to using 4 spaces, although I couldn't really care less.

> +  # Probe for the actual type.
> +  send_gdb "print &foo::overload1arg($types($type))\n"
> +  gdb_expect {

Can we use test_gdb_multiple, here?

Personally, I think we would have less damage if we just poisoned
send_gdb, accepting to give up some testcases that we wouldn't be
able to write differently, over trying to make sure we only use them
when necessary.

-- 
Joel