Re: [PATCH 151/203] Split out some Ada type resolution code

Joel Brobecker
2021-01-03T07:46:59Z

Hi Tom,

On Fri, Jan 01, 2021 at 02:46:31PM -0700, Tom Tromey wrote:
> This splits some Ada type resolution code out of resolve_subexp into
> new functions that can be reused.
>
> gdb/ChangeLog
> 2021-01-01 Tom Tromey <tom@tromey.com>
>
> * ada-lang.h (ada_find_operator_symbol, ada_resolve_funcall)
> (ada_resolve_variable): Declare.
> * ada-lang.c (ada_find_operator_symbol, ada_resolve_funcall)
> (ada_resolve_variable): New functions.
> (resolve_subexp): Update.

Could this patch go in early, and independently of this patch series
(or perhaps right after the first third of the patches splitting
the various eval routines go in)?

As far as I can tell, all it needs is documentation of the new routines
in ada-lang.h. Other than that, this patch looks like a nice little
improvement of the code's organization.

Thanks!

> ---
> gdb/ChangeLog | 8 ++
> gdb/ada-lang.c | 266 ++++++++++++++++++++++++++++---------------------
> gdb/ada-lang.h | 18 ++++
> 3 files changed, 176 insertions(+), 116 deletions(-)
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index e2c9de69b65..50b82e6bcf9 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -3452,6 +3452,132 @@ See set/show multiple-symbol."));
> return n_chosen;
> }
>
> +/* See ada-lang.h. */
> +
> +block_symbol
> +ada_find_operator_symbol (enum exp_opcode op, int parse_completion,
> + int nargs, value *argvec[])
> +{
> + if (possible_user_operator_p (op, argvec))
> + {
> + std::vector<struct block_symbol> candidates;
> + int n_candidates = ada_lookup_symbol_list (ada_decoded_op_name (op),
> + NULL, VAR_DOMAIN,
> + &candidates);
> + int i = ada_resolve_function (candidates.data (), n_candidates, argvec,
> + nargs, ada_decoded_op_name (op), NULL,
> + parse_completion);
> + if (i >= 0)
> + return candidates[i];
> + }
> + return {};
> +}
> +
> +/* See ada-lang.h. */
> +
> +block_symbol
> +ada_resolve_funcall (struct symbol *sym, const struct block *block,
> + struct type *context_type,
> + int parse_completion,
> + int nargs, value *argvec[],
> + innermost_block_tracker *tracker)
> +{
> + std::vector<struct block_symbol> candidates;
> + int n_candidates = ada_lookup_symbol_list (sym->linkage_name (), block,
> + VAR_DOMAIN, &candidates);
> +
> + int i;
> + if (n_candidates == 1)
> + i = 0;
> + else
> + {
> + i = ada_resolve_function (candidates.data (), n_candidates,
> + argvec, nargs,
> + sym->linkage_name (),
> + context_type, parse_completion);
> + if (i < 0)
> + error (_("Could not find a match for %s"), sym->print_name ());
> + }
> +
> + tracker->update (candidates[i]);
> + return candidates[i];
> +}
> +
> +/* See ada-lang.h. */
> +
> +block_symbol
> +ada_resolve_variable (struct symbol *sym, const struct block *block,
> + struct type *context_type,
> + int parse_completion,
> + int deprocedure_p,
> + innermost_block_tracker *tracker)
> +{
> + std::vector<struct block_symbol> candidates;
> + int n_candidates = ada_lookup_symbol_list (sym->linkage_name (),
> + block, VAR_DOMAIN,
> + &candidates);
> +
> + if (n_candidates > 1)
> + {
> + /* Types tend to get re-introduced locally, so if there are any
> + local symbols that are not types, first filter out all
> + types. */
> + int j;
> + for (j = 0; j < n_candidates; j += 1)
> + switch (SYMBOL_CLASS (candidates[j].symbol))
> + {
> + case LOC_REGISTER:
> + case LOC_ARG:
> + case LOC_REF_ARG:
> + case LOC_REGPARM_ADDR:
> + case LOC_LOCAL:
> + case LOC_COMPUTED:
> + goto FoundNonType;
> + default:
> + break;
> + }
> + FoundNonType:
> + if (j < n_candidates)
> + {
> + j = 0;
> + while (j < n_candidates)
> + {
> + if (SYMBOL_CLASS (candidates[j].symbol) == LOC_TYPEDEF)
> + {
> + candidates[j] = candidates[n_candidates - 1];
> + n_candidates -= 1;
> + }
> + else
> + j += 1;
> + }
> + }
> + }
> +
> + int i;
> + if (n_candidates == 0)
> + error (_("No definition found for %s"), sym->print_name ());
> + else if (n_candidates == 1)
> + i = 0;
> + else if (deprocedure_p
> + && !is_nonfunction (candidates.data (), n_candidates))
> + {
> + i = ada_resolve_function (candidates.data (), n_candidates, NULL, 0,
> + sym->linkage_name (),
> + context_type, parse_completion);
> + if (i < 0)
> + error (_("Could not find a match for %s"), sym->print_name ());
> + }
> + else
> + {
> + printf_filtered (_("Multiple matches for %s\n"), sym->print_name ());
> + user_select_syms (candidates.data (), n_candidates, 1);
> + i = 0;
> + }
> +
> + tracker->update (candidates[i]);
> + return candidates[i];
> +}
> +
> /* Resolve the operator of the subexpression beginning at
> position *POS of *EXPP. "Resolving" consists of replacing
> the symbols that have undefined namespaces in OP_VAR_VALUE nodes
> @@ -3640,77 +3766,13 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
> case OP_VAR_VALUE:
> if (SYMBOL_DOMAIN (exp->elts[pc + 2].symbol) == UNDEF_DOMAIN)
> {
> - std::vector<struct block_symbol> candidates;
> - int n_candidates;
> -
> - n_candidates =
> - ada_lookup_symbol_list (exp->elts[pc + 2].symbol->linkage_name (),
> - exp->elts[pc + 1].block, VAR_DOMAIN,
> - &candidates);
> -
> - if (n_candidates > 1)
> - {
> - /* Types tend to get re-introduced locally, so if there
> - are any local symbols that are not types, first filter
> - out all types. */
> - int j;
> - for (j = 0; j < n_candidates; j += 1)
> - switch (SYMBOL_CLASS (candidates[j].symbol))
> - {
> - case LOC_REGISTER:
> - case LOC_ARG:
> - case LOC_REF_ARG:
> - case LOC_REGPARM_ADDR:
> - case LOC_LOCAL:
> - case LOC_COMPUTED:
> - goto FoundNonType;
> - default:
> - break;
> - }
> - FoundNonType:
> - if (j < n_candidates)
> - {
> - j = 0;
> - while (j < n_candidates)
> - {
> - if (SYMBOL_CLASS (candidates[j].symbol) == LOC_TYPEDEF)
> - {
> - candidates[j] = candidates[n_candidates - 1];
> - n_candidates -= 1;
> - }
> - else
> - j += 1;
> - }
> - }
> - }
> -
> - if (n_candidates == 0)
> - error (_("No definition found for %s"),
> - exp->elts[pc + 2].symbol->print_name ());
> - else if (n_candidates == 1)
> - i = 0;
> - else if (deprocedure_p
> - && !is_nonfunction (candidates.data (), n_candidates))
> - {
> - i = ada_resolve_function
> - (candidates.data (), n_candidates, NULL, 0,
> - exp->elts[pc + 2].symbol->linkage_name (),
> - context_type, parse_completion);
> - if (i < 0)
> - error (_("Could not find a match for %s"),
> - exp->elts[pc + 2].symbol->print_name ());
> - }
> - else
> - {
> - printf_filtered (_("Multiple matches for %s\n"),
> - exp->elts[pc + 2].symbol->print_name ());
> - user_select_syms (candidates.data (), n_candidates, 1);
> - i = 0;
> - }
> -
> - exp->elts[pc + 1].block = candidates[i].block;
> - exp->elts[pc + 2].symbol = candidates[i].symbol;
> - tracker->update (candidates[i]);
> + block_symbol resolved
> + = ada_resolve_variable (exp->elts[pc + 2].symbol,
> + exp->elts[pc + 1].block,
> + context_type, parse_completion,
> + deprocedure_p, tracker);
> + exp->elts[pc + 1].block = resolved.block;
> + exp->elts[pc + 2].symbol = resolved.symbol;
> }
>
> if (deprocedure_p
> @@ -3729,31 +3791,14 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
> if (exp->elts[pc + 3].opcode == OP_VAR_VALUE
> && SYMBOL_DOMAIN (exp->elts[pc + 5].symbol) == UNDEF_DOMAIN)
> {
> - std::vector<struct block_symbol> candidates;
> - int n_candidates;
> -
> - n_candidates =
> - ada_lookup_symbol_list (exp->elts[pc + 5].symbol->linkage_name (),
> - exp->elts[pc + 4].block, VAR_DOMAIN,
> - &candidates);
> -
> - if (n_candidates == 1)
> - i = 0;
> - else
> - {
> - i = ada_resolve_function
> - (candidates.data (), n_candidates,
> - argvec, nargs,
> - exp->elts[pc + 5].symbol->linkage_name (),
> - context_type, parse_completion);
> - if (i < 0)
> - error (_("Could not find a match for %s"),
> - exp->elts[pc + 5].symbol->print_name ());
> - }
> -
> - exp->elts[pc + 4].block = candidates[i].block;
> - exp->elts[pc + 5].symbol = candidates[i].symbol;
> - tracker->update (candidates[i]);
> + block_symbol resolved
> + = ada_resolve_funcall (exp->elts[pc + 5].symbol,
> + exp->elts[pc + 4].block,
> + context_type, parse_completion,
> + nargs, argvec,
> + tracker);
> + exp->elts[pc + 4].block = resolved.block;
> + exp->elts[pc + 5].symbol = resolved.symbol;
> }
> }
> break;
> @@ -3778,27 +3823,16 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
> case UNOP_PLUS:
> case UNOP_LOGICAL_NOT:
> case UNOP_ABS:
> - if (possible_user_operator_p (op, argvec))
> - {
> - std::vector<struct block_symbol> candidates;
> - int n_candidates;
> -
> - n_candidates =
> - ada_lookup_symbol_list (ada_decoded_op_name (op),
> - NULL, VAR_DOMAIN,
> - &candidates);
> -
> - i = ada_resolve_function (candidates.data (), n_candidates, argvec,
> - nargs, ada_decoded_op_name (op), NULL,
> - parse_completion);
> - if (i < 0)
> - break;
> + {
> + block_symbol found = ada_find_operator_symbol (op, parse_completion,
> + nargs, argvec);
> + if (found.symbol == nullptr)
> + break;
>
> - replace_operator_with_call (expp, pc, nargs, 1,
> - candidates[i].symbol,
> - candidates[i].block);
> - exp = expp->get ();
> - }
> + replace_operator_with_call (expp, pc, nargs, 1,
> + found.symbol, found.block);
> + exp = expp->get ();
> + }
> break;
>
> case OP_TYPE:
> diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
> index dbf45a84928..8333def6280 100644
> --- a/gdb/ada-lang.h
> +++ b/gdb/ada-lang.h
> @@ -386,4 +386,22 @@ extern void print_ada_task_info (struct ui_out *uiout,
> const char *taskno_str,
> struct inferior *inf);
>
> +extern block_symbol ada_find_operator_symbol (enum exp_opcode op,
> + int parse_completion,
> + int nargs, value *argvec[]);
> +
> +extern block_symbol ada_resolve_funcall (struct symbol *sym,
> + const struct block *block,
> + struct type *context_type,
> + int parse_completion,
> + int nargs, value *argvec[],
> + innermost_block_tracker *tracker);
> +
> +extern block_symbol ada_resolve_variable (struct symbol *sym,
> + const struct block *block,
> + struct type *context_type,
> + int parse_completion,
> + int deprocedure_p,
> + innermost_block_tracker *tracker);
> +
> #endif
> --
> 2.26.2

--
Joel