Re: [RFA 10/11] Use a std::vector for ada_exceptions_list

Pedro Alves
2017-09-28T10:20:45Z

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change ada_exceptions_list to return a std::vector and fix up the
> users. This allows removing a cleanup in MI.

Looks good to me with the nits below addressed.

> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 64f1a33..cafba2d 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -62,6 +62,7 @@
> #include "cli/cli-utils.h"
> #include "common/function-view.h"
> #include "common/byte-vector.h"
> +#include <algorithm>
>
> /* Define whether or not the C operator '/' truncates towards zero for
> differently signed operands (truncation direction is undefined in C).
> @@ -13121,23 +13122,23 @@ ada_is_non_standard_exception_sym (struct symbol *sym)
> The comparison is determined first by exception name, and then
> by exception address. */

This comment talks about qsort. It should be updated to mention
std::sort instead, since the logic is different.

>
> -static int
> -compare_ada_exception_info (const void *a, const void *b)
> +bool
> +ada_exc_info::operator< (const ada_exc_info &other)
> {
> - const struct ada_exc_info *exc_a = (struct ada_exc_info *) a;
> - const struct ada_exc_info *exc_b = (struct ada_exc_info *) b;
> int result;
>
> - result = strcmp (exc_a->name, exc_b->name);
> - if (result != 0)
> - return result;
> -
> - if (exc_a->addr < exc_b->addr)
> - return -1;
> - if (exc_a->addr > exc_b->addr)
> - return 1;
> + result = strcmp (name, other.name);
> + if (result < 0)
> + return true;
> + if (result == 0 && addr < other.addr)
> + return true;
> + return false;
> +}
>
> - return 0;
> +bool
> +ada_exc_info::operator== (const ada_exc_info &other)
> +{
> + return strcmp (name, other.name) == 0 && addr == other.addr;

I'd swap the comparisons to put the cheap addr comparison first.

> if (regexp != NULL)
> printf_filtered
> @@ -13404,10 +13386,8 @@ info_exceptions_command (char *regexp, int from_tty)
> else
> printf_filtered (_("All defined Ada exceptions:\n"));
>
> - for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
> - printf_filtered ("%s: %s\n", info->name, paddress (gdbarch, info->addr));
> -
> - do_cleanups (cleanup);
> + for (ada_exc_info &info : exceptions)
> + printf_filtered ("%s: %s\n", info.name, paddress (gdbarch, info.addr));

I'd write 'const ada_exc_info &', just for general use-const-if-possible
reasons.

> - for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
> + for (ada_exc_info &info : exceptions)

Ditto.

Thanks,
Pedro Alves