[RFA/commit] Rename la_get_symbol_name_match_p into la_get_symbol_name_cmp

[RFA/commit] Rename la_get_symbol_name_match_p into la_get_symbol_name_cmp

[Joel Brobecker]

Hello,

Re: [patch] Fix the 2012-01-26 regression by la_get_symbol_name_match_p
    [Re: Crash regression gdb.cp/no-dmgl-verbose.exp]
    http://www.sourceware.org/ml/gdb-patches/2012-01/msg00956.html

The la_get_symbol_name_match_p language hook was poorly named, as
it suggested that the function should return nonzero if the names
match, whereas it is the exact opposite.  This patch therefore
renames the hook and associated typedef, as well some of the code
that uses that hook.

gdb/ChangeLog:

        * language.h (symbol_name_cmp_ftype): Renames
        symbol_name_match_p_ftype.  Update documentation.
        (struct language_defn) [la_get_symbol_name_cmp]: Renames
        la_get_symbol_name_match_p.  Update documentation.
        * ada-lang.c (ada_get_symbol_name_cmp): Renames
        ada_get_symbol_name_match_p.
        (ada_language_defn): Replace ada_get_symbol_name_match_p by
        ada_get_symbol_name_cmp.
        * linespec.c (struct symbol_matcher_data) [symbol_name_cmp]:
        Renames symbol_name_match_p.
        (iterate_name_matcher, iterate_over_all_matching_symtabs):
        Update.

Tested on x86_64-linux, no regression. I'd like to commit that, unless
there are better naming suggestions?

Thanks,
-- 
Joel

---
 gdb/ada-lang.c |    6 +++---
 gdb/language.h |   15 +++++++++------
 gdb/linespec.c |   10 +++++-----
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 94178d3..07326e3 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12385,8 +12385,8 @@ static const struct exp_descriptor ada_exp_descriptor = {
 /* Implement the "la_get_symbol_name_match_p" language_defn method
    for Ada.  */
 
-static symbol_name_match_p_ftype
-ada_get_symbol_name_match_p (const char *lookup_name)
+static symbol_name_cmp_ftype
+ada_get_symbol_name_cmp (const char *lookup_name)
 {
   if (should_use_wild_match (lookup_name))
     return wild_match;
@@ -12430,7 +12430,7 @@ const struct language_defn ada_language_defn = {
   ada_print_array_index,
   default_pass_by_reference,
   c_get_string,
-  ada_get_symbol_name_match_p,	/* la_get_symbol_name_match_p */
+  ada_get_symbol_name_cmp,	/* la_get_symbol_name_match_p */
   ada_iterate_over_symbols,
   LANG_MAGIC
 };
diff --git a/gdb/language.h b/gdb/language.h
index 7a1bcb7..7bfeffe 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -136,15 +136,19 @@ struct language_arch_info
   struct type *bool_type_default;
 };
 
-/* A pointer to a function expected to return nonzero if
+/* A pointer to a function expected to return zero iff
    SYMBOL_SEARCH_NAME matches the given LOOKUP_NAME.
+   Return nonzero otherwise.
+
+   Despite what the name might suggest, there is no ordering involved.
+   If the names do not match, any non-zero return value is correct.
 
    SYMBOL_SEARCH_NAME should be a symbol's "search" name.
    LOOKUP_NAME should be the name of an entity after it has been
    transformed for lookup.  */
 
-typedef int (*symbol_name_match_p_ftype) (const char *symbol_search_name,
-					  const char *lookup_name);
+typedef int (*symbol_name_cmp_ftype) (const char *symbol_search_name,
+				      const char *lookup_name);
 
 /* Structure tying together assorted information about a language.  */
 
@@ -328,14 +332,13 @@ struct language_defn
     void (*la_get_string) (struct value *value, gdb_byte **buffer, int *length,
 			   struct type **chartype, const char **charset);
 
-    /* Return a pointer to the function that should be used to match
+    /* Return a pointer to the function that should be used to compare
        a symbol name against LOOKUP_NAME. This is mostly for languages
        such as Ada where the matching algorithm depends on LOOKUP_NAME.
 
        This field may be NULL, in which case strcmp_iw will be used
        to perform the matching.  */
-    symbol_name_match_p_ftype (*la_get_symbol_name_match_p)
-      (const char *lookup_name);
+    symbol_name_cmp_ftype (*la_get_symbol_name_cmp) (const char *lookup_name);
 
     /* Find all symbols in the current program space matching NAME in
        DOMAIN, according to this language's rules.
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 50ebf6f..034ffa8 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -329,7 +329,7 @@ struct symbol_matcher_data
   const char *lookup_name;
 
   /* The routine to be used for comparison.  */
-  symbol_name_match_p_ftype symbol_name_match_p;
+  symbol_name_cmp_ftype symbol_name_cmp;
 };
 
 /* A helper for iterate_over_all_matching_symtabs that is passed as a
@@ -340,7 +340,7 @@ iterate_name_matcher (const char *name, void *d)
 {
   const struct symbol_matcher_data *data = d;
 
-  if (data->symbol_name_match_p (name, data->lookup_name) == 0)
+  if (data->symbol_name_cmp (name, data->lookup_name) == 0)
     return 1;
   return 0;
 }
@@ -362,9 +362,9 @@ iterate_over_all_matching_symtabs (const char *name,
   struct symbol_matcher_data matcher_data;
 
   matcher_data.lookup_name = name;
-  matcher_data.symbol_name_match_p =
-    current_language->la_get_symbol_name_match_p != NULL
-    ? current_language->la_get_symbol_name_match_p (name)
+  matcher_data.symbol_name_cmp =
+    current_language->la_get_symbol_name_cmp != NULL
+    ? current_language->la_get_symbol_name_cmp (name)
     : strcmp_iw;
 
   ALL_PSPACES (pspace)
-- 
1.7.1

Re: [RFA/commit] Rename la_get_symbol_name_match_p into la_get_symbol_name_cmp

[Jan Kratochvil]

On Mon, 30 Jan 2012 07:55:22 +0100, Joel Brobecker wrote:
> unless there are better naming suggestions?

Maybe "la_get_symbol_name_no_match"?  As you note yourself:

> +   Despite what the name might suggest, there is no ordering involved.
> +   If the names do not match, any non-zero return value is correct.



>  /* Implement the "la_get_symbol_name_match_p" language_defn method
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
Forgotten unchanged string.

[...]
> -  ada_get_symbol_name_match_p,	/* la_get_symbol_name_match_p */
> +  ada_get_symbol_name_cmp,	/* la_get_symbol_name_match_p */
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^
Forgotten unchanged string.  I did not grep the files outside of this diff.


Thanks,
Jan

Re: [RFA/commit] Rename la_get_symbol_name_match_p into la_get_symbol_name_cmp

[Joel Brobecker]

Thanks for spotting the missing changes...

> On Mon, 30 Jan 2012 07:55:22 +0100, Joel Brobecker wrote:
> > unless there are better naming suggestions?
> 
> Maybe "la_get_symbol_name_no_match"?  As you note yourself:

It's an interesting suggestion, but it introduces double-negatives,
and I'd rather not do that. Not that it's critically important,
I'll use whatever name is prefered by most.

I'll wait a day or two to collect feedback, and send a new patch.

Thanks again,
-- 
Joel

Re: [RFA/commit] Rename la_get_symbol_name_match_p into la_get_symbol_name_cmp

[Pedro Alves]

On 01/30/2012 07:13 AM, Joel Brobecker wrote:
> Thanks for spotting the missing changes...
> 
>> On Mon, 30 Jan 2012 07:55:22 +0100, Joel Brobecker wrote:
>>> unless there are better naming suggestions?
>>
>> Maybe "la_get_symbol_name_no_match"?  As you note yourself:
> 
> It's an interesting suggestion, but it introduces double-negatives,
> and I'd rather not do that.

Did you consider keeping the name, but reverse the return logic to
be what one would expect instead (return true/false for match)?

-- 
Pedro Alves

Re: [RFA/commit] Rename la_get_symbol_name_match_p into la_get_symbol_name_cmp

[Joel Brobecker]

> Did you consider keeping the name, but reverse the return logic to
> be what one would expect instead (return true/false for match)?

I did. But the cost is that we cannot use strcmp* as default matching
routines. We end up needing a wrapper, which isn't great and also
costs us an extra function call.

-- 
Joel

Re: [RFA/commit] Rename la_get_symbol_name_match_p into la_get_symbol_name_cmp

[Pedro Alves]

On 01/30/2012 11:36 AM, Joel Brobecker wrote:
>> Did you consider keeping the name, but reverse the return logic to
>> be what one would expect instead (return true/false for match)?
> 
> I did. But the cost is that we cannot use strcmp* as default matching
> routines. We end up needing a wrapper, which isn't great and also
> costs us an extra function call.

Okay.  Not sure whether the wrapper/call makes a real difference,
but in any case, la_get_symbol_name_cmp makes sense and looks fine
to me.  (I find "no_match" confusing.)

-- 
Pedro Alves

FYI: Rename la_get_symbol_name_match_p into la_get_symbol_name_cmp

[Joel Brobecker]

Hello,

Re: http://www.sourceware.org/ml/gdb-patches/2012-01/msg00977.html

Here is a new patch, which should fix the various issues reported
by Jan in my first patch.  To make sure I didn't forget any instances,
I simply used search-and-replace (s/symbol_name_match_p/symbol_name_cmp/).

gdb/ChangeLog:

        * language.h (symbol_name_cmp_ftype): Renames
        symbol_name_match_p_ftype.
        (struct language_defn)[la_get_symbol_name_cmp]: Renames
        la_get_symbol_name_match_p.
        * ada-lang.c (ada_get_symbol_name_cmp): Renames
        ada_get_symbol_name_match_p.  Update comment.
        (ada_language_defn)[la_get_symbol_name_cmp]: Update value.
        * linespec.c (struct symbol_matcher_data)[symbol_name_cmp]:
        Renames symbol_name_match_p.  Update field type.
        (iterate_name_matcher, iterate_over_all_matching_symtabs): Adjust.
        * c-lang.c, d-lang.c, f-lang.c, jv-lang.c, m2-lang.c, objc-lang.c,
        opencl-lang.c, p-lang.c: Replace "la_get_symbol_name_match_p" by
        "la_get_symbol_name_cmp" in comments.
        * language.c: Likewise.

Tested on x86_64-linux.  Will apply sometime next week if there are
no objection.

---
 gdb/ada-lang.c    |    8 ++++----
 gdb/c-lang.c      |    8 ++++----
 gdb/d-lang.c      |    2 +-
 gdb/f-lang.c      |    2 +-
 gdb/jv-lang.c     |    2 +-
 gdb/language.c    |    6 +++---
 gdb/language.h    |    5 ++---
 gdb/linespec.c    |   10 +++++-----
 gdb/m2-lang.c     |    2 +-
 gdb/objc-lang.c   |    2 +-
 gdb/opencl-lang.c |    2 +-
 gdb/p-lang.c      |    2 +-
 12 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 3e9070f..ef36cbc 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12385,11 +12385,11 @@ static const struct exp_descriptor ada_exp_descriptor = {
   ada_evaluate_subexp
 };
 
-/* Implement the "la_get_symbol_name_match_p" language_defn method
+/* Implement the "la_get_symbol_name_cmp" language_defn method
    for Ada.  */
 
-static symbol_name_match_p_ftype
-ada_get_symbol_name_match_p (const char *lookup_name)
+static symbol_name_cmp_ftype
+ada_get_symbol_name_cmp (const char *lookup_name)
 {
   if (should_use_wild_match (lookup_name))
     return wild_match;
@@ -12433,7 +12433,7 @@ const struct language_defn ada_language_defn = {
   ada_print_array_index,
   default_pass_by_reference,
   c_get_string,
-  ada_get_symbol_name_match_p,	/* la_get_symbol_name_match_p */
+  ada_get_symbol_name_cmp,	/* la_get_symbol_name_cmp */
   ada_iterate_over_symbols,
   LANG_MAGIC
 };
diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 6e92fb9..1e89d09 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -863,7 +863,7 @@ const struct language_defn c_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   c_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
@@ -986,7 +986,7 @@ const struct language_defn cplus_language_defn =
   default_print_array_index,
   cp_pass_by_reference,
   c_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
@@ -1027,7 +1027,7 @@ const struct language_defn asm_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   c_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
@@ -1073,7 +1073,7 @@ const struct language_defn minimal_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   c_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
diff --git a/gdb/d-lang.c b/gdb/d-lang.c
index 29dcfe4..80504b4 100644
--- a/gdb/d-lang.c
+++ b/gdb/d-lang.c
@@ -272,7 +272,7 @@ static const struct language_defn d_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   c_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   NULL,
   LANG_MAGIC
 };
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 5f15402..389f08b 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -309,7 +309,7 @@ const struct language_defn f_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   default_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
diff --git a/gdb/jv-lang.c b/gdb/jv-lang.c
index e6fcaee..17c013f 100644
--- a/gdb/jv-lang.c
+++ b/gdb/jv-lang.c
@@ -1196,7 +1196,7 @@ const struct language_defn java_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   default_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
diff --git a/gdb/language.c b/gdb/language.c
index d92830f..49ba21a 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -933,7 +933,7 @@ const struct language_defn unknown_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   default_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
@@ -976,7 +976,7 @@ const struct language_defn auto_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   default_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
@@ -1017,7 +1017,7 @@ const struct language_defn local_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   default_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
diff --git a/gdb/language.h b/gdb/language.h
index d4cce95..a47a44d 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -143,7 +143,7 @@ struct language_arch_info
    LOOKUP_NAME should be the name of an entity after it has been
    transformed for lookup.  */
 
-typedef int (*symbol_name_match_p_ftype) (const char *symbol_search_name,
+typedef int (*symbol_name_cmp_ftype) (const char *symbol_search_name,
 					  const char *lookup_name);
 
 /* Structure tying together assorted information about a language.  */
@@ -334,8 +334,7 @@ struct language_defn
 
        This field may be NULL, in which case strcmp_iw will be used
        to perform the matching.  */
-    symbol_name_match_p_ftype (*la_get_symbol_name_match_p)
-      (const char *lookup_name);
+    symbol_name_cmp_ftype (*la_get_symbol_name_cmp) (const char *lookup_name);
 
     /* Find all symbols in the current program space matching NAME in
        DOMAIN, according to this language's rules.
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 4e42750..d7f3139 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -329,7 +329,7 @@ struct symbol_matcher_data
   const char *lookup_name;
 
   /* The routine to be used for comparison.  */
-  symbol_name_match_p_ftype symbol_name_match_p;
+  symbol_name_cmp_ftype symbol_name_cmp;
 };
 
 /* A helper for iterate_over_all_matching_symtabs that is passed as a
@@ -340,7 +340,7 @@ iterate_name_matcher (const char *name, void *d)
 {
   const struct symbol_matcher_data *data = d;
 
-  if (data->symbol_name_match_p (name, data->lookup_name) == 0)
+  if (data->symbol_name_cmp (name, data->lookup_name) == 0)
     return 1; /* Expand this symbol's symbol table.  */
   return 0; /* Skip this symbol.  */
 }
@@ -362,9 +362,9 @@ iterate_over_all_matching_symtabs (const char *name,
   struct symbol_matcher_data matcher_data;
 
   matcher_data.lookup_name = name;
-  matcher_data.symbol_name_match_p =
-    current_language->la_get_symbol_name_match_p != NULL
-    ? current_language->la_get_symbol_name_match_p (name)
+  matcher_data.symbol_name_cmp =
+    current_language->la_get_symbol_name_cmp != NULL
+    ? current_language->la_get_symbol_name_cmp (name)
     : strcmp_iw;
 
   ALL_PSPACES (pspace)
diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c
index d7b16f4..9cd27c9 100644
--- a/gdb/m2-lang.c
+++ b/gdb/m2-lang.c
@@ -401,7 +401,7 @@ const struct language_defn m2_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   default_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 94951fe..742916e 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -540,7 +540,7 @@ const struct language_defn objc_language_defn = {
   default_print_array_index,
   default_pass_by_reference,
   default_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
index d779591..80f978c 100644
--- a/gdb/opencl-lang.c
+++ b/gdb/opencl-lang.c
@@ -1024,7 +1024,7 @@ const struct language_defn opencl_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   c_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
diff --git a/gdb/p-lang.c b/gdb/p-lang.c
index b634b99..2c829c9 100644
--- a/gdb/p-lang.c
+++ b/gdb/p-lang.c
@@ -459,7 +459,7 @@ const struct language_defn pascal_language_defn =
   default_print_array_index,
   default_pass_by_reference,
   default_get_string,
-  NULL,				/* la_get_symbol_name_match_p */
+  NULL,				/* la_get_symbol_name_cmp */
   iterate_over_symbols,
   LANG_MAGIC
 };
-- 
1.7.1

Re: FYI: Rename la_get_symbol_name_match_p into la_get_symbol_name_cmp

[Joel Brobecker]

> gdb/ChangeLog:
> 
>         * language.h (symbol_name_cmp_ftype): Renames
>         symbol_name_match_p_ftype.
>         (struct language_defn)[la_get_symbol_name_cmp]: Renames
>         la_get_symbol_name_match_p.
>         * ada-lang.c (ada_get_symbol_name_cmp): Renames
>         ada_get_symbol_name_match_p.  Update comment.
>         (ada_language_defn)[la_get_symbol_name_cmp]: Update value.
>         * linespec.c (struct symbol_matcher_data)[symbol_name_cmp]:
>         Renames symbol_name_match_p.  Update field type.
>         (iterate_name_matcher, iterate_over_all_matching_symtabs): Adjust.
>         * c-lang.c, d-lang.c, f-lang.c, jv-lang.c, m2-lang.c, objc-lang.c,
>         opencl-lang.c, p-lang.c: Replace "la_get_symbol_name_match_p" by
>         "la_get_symbol_name_cmp" in comments.
>         * language.c: Likewise.

FYI: Checked in.

-- 
Joel