From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9604 invoked by alias); 28 Sep 2008 22:01:44 -0000 Received: (qmail 9587 invoked by uid 22791); 28 Sep 2008 22:01:40 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sun, 28 Sep 2008 22:01:05 +0000 Received: (qmail 31127 invoked from network); 28 Sep 2008 22:01:03 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 28 Sep 2008 22:01:03 -0000 From: Pedro Alves To: gdb-patches@sourceware.org, tromey@redhat.com Subject: Re: RFA: PR 2484: completion of macro names Date: Sun, 28 Sep 2008 22:01:00 -0000 User-Agent: KMail/1.9.9 References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200809282301.17191.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-09/txt/msg00551.txt.bz2 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 FOO, 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