Re: RFA: PR 2484: completion of macro names

Pedro Alves
2008-09-28T22:01:44Z

Hi Tom,

Thanks a lot for doing this. I'd love to get this feature in, and
it seems no one has reviewed it yet, so here's my try at grasping
it. Please see comments below.

On Tuesday 29 July 2008 21:38:48, Tom Tromey wrote:
> +static int
> +foreach_macro_in_scope (splay_tree_node node, void *info)
> {
> - /* Note that we pass in the address of 'fn' because, pedantically
> - speaking, we can't necessarily cast a pointer-to-function to a
> - void*. */
> - splay_tree_foreach (table->definitions, foreach_macro, &fn);
> + struct macro_for_each_data *datum = (struct macro_for_each_data *) info;
> + struct macro_key *key = (struct macro_key *) node->key;
> + struct macro_definition *def = (struct macro_definition *) node->value;
> +
> + if (compare_locations (key->start_file, key->start_line,
> + datum->file, datum->line) < 0)
> + (*datum->fn) (key->name, def, datum->user_data);
> + return 0;
> +}

This matches macros that have been undefined before the
current line, was that intended? E.g.,

define FOO 1
#define BAR 2
#undef FOO

int main ()
{
return 0; <<< stop here
}

This will still complete p F<tab>OO, but then
if the user tries to evaluate it the user will
get, correcty:

(gdb) p FOO
No symbol "FOO" in current context.
(gdb)

I think you want something along the lines of:

if (compare_locations (key->start_file, key->start_line,
datum->file, datum->line) < 0
&& (key->end_file == 0
|| compare_locations (key->end_file, key->end_line,
datum->file, datum->line) >= 0))
(*datum->fn) (key->name, def, datum->user_data);

?

> + /* Add any macros visible in the default scope. Note that this
> + may yield the occasional wrong result, because an expression
> + might be evaluated in a scope other than the default. There
> + does not seem to be a way to detect this at completion time. */
> + scope = default_macro_scope ();
> + if (scope)
> + {
> + macro_for_each_in_scope (scope->file, scope->line,
> + add_macro_name, &datum);
> + xfree (scope);
> + }

I'm a bonehead, and don't follow this :-). Could you show/add here
an example of one such case, that would matter for completion?

> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -153,6 +153,9 @@ struct language_defn
> /* Multi-dimensional array ordering */
> enum array_ordering la_array_ordering;
>
> + /* True if this language supports C-like macro expansion. */
> + unsigned int la_macro_expansion : 1;
> +

We're already being inconsistent by using enum as flags in this
structure (e.g., case_sensitivity), or char's as flags (c_style_arrays).
This adds yet a third way to do things. IMVHO, we should strive for
consistency. Since several other language flags are enums, and
since there aren't that many languages that we have to consider
space savings here, making this an enum as well would make these,

> case_sensitive_on,
> array_row_major,
> + 0,
> &exp_descriptor_modula2,

... easier to read. Again, this is a IMVHO.

> --- a/gdb/jv-lang.c
> +++ b/gdb/jv-lang.c
> @@ -1057,6 +1057,7 @@ const struct language_defn java_language_defn =
> type_check_off,
> case_sensitive_on,
> array_row_major,
> + 0,
> &exp_descriptor_java,
> java_parse,
> java_error,

Out of curiosity --- can one use the C preprocessor with gcj? Will
gcj output macro debug info with -g3 in that case? (Or any
other non-C frontend for the matter)

--
Pedro Alves