diff mbox

[14/40] Introduce CP_OPERATOR_STR/CP_OPERATOR_LEN and use throughout

Message ID 1496406158-12663-15-git-send-email-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves June 2, 2017, 12:22 p.m. UTC 
  I ran into LENGTH_OF_OPERATOR in cp-support.c and wanted to use it
elsewhere, so I moved it to cp-support.h.  Since there's already
CP_ANONYMOUS_NAMESPACE_STR/CP_ANONYMOUS_NAMESPACE_LEN there, I
followed the same naming pattern for the new symbols.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* c-exp.y (operator_stoken): Use CP_OPERATOR_LEN and
	CP_OPERATOR_STR.
	* c-typeprint.c (is_type_conversion_operator): Use
	CP_OPERATOR_STR.
	* cp-support.c (LENGTH_OF_OPERATOR): Delete.
	(cp_find_first_component_aux): Use CP_OPERATOR_STR and
	CP_OPERATOR_LEN.
	* cp-support.h (CP_OPERATOR_STR, CP_OPERATOR_LEN): New.
	* gnu-v2-abi.c (gnuv2_is_operator_name): Use CP_OPERATOR_STR.
	* gnu-v3-abi.c (gnuv3_is_operator_name): Use CP_OPERATOR_STR.
	* linespec.c (linespec_lexer_lex_string): Use CP_OPERATOR_LEN and
	CP_OPERATOR_STR.
	* location.c: Include "cp-support.h".
	(explicit_location_lex_one): Use CP_OPERATOR_LEN and
	CP_OPERATOR_STR.
	* symtab.c (operator_chars): Use CP_OPERATOR_STR and
	CP_OPERATOR_LEN.
---
 gdb/c-exp.y       | 5 ++---
 gdb/c-typeprint.c | 2 +-
 gdb/cp-support.c  | 9 ++-------
 gdb/cp-support.h  | 8 ++++++++
 gdb/gnu-v2-abi.c  | 2 +-
 gdb/gnu-v3-abi.c  | 2 +-
 gdb/linespec.c    | 5 ++---
 gdb/location.c    | 5 +++--
 gdb/symtab.c      | 4 ++--
 9 files changed, 22 insertions(+), 20 deletions(-)

Comments

Keith Seitz July 14, 2017, 6:04 p.m. UTC | #1 
On 06/02/2017 05:22 AM, Pedro Alves wrote:
> I ran into LENGTH_OF_OPERATOR in cp-support.c and wanted to use it
> elsewhere, so I moved it to cp-support.h.  Since there's already
> CP_ANONYMOUS_NAMESPACE_STR/CP_ANONYMOUS_NAMESPACE_LEN there, I
> followed the same naming pattern for the new symbols.

Looks pretty straightforward (even?) to me, but I do notice that there are still one or two places that could be updated:

c-exp.y:2281:    {"operator", OPERATOR, OP_NULL, FLAG_CXX},
cp-name-parser.y:1839:      if (strncmp (tokstart, "operator", 8) == 0)

Keith
Pedro Alves July 17, 2017, 2:55 p.m. UTC | #2 
On 07/14/2017 07:04 PM, Keith Seitz wrote:
> On 06/02/2017 05:22 AM, Pedro Alves wrote:
>> I ran into LENGTH_OF_OPERATOR in cp-support.c and wanted to use it
>> elsewhere, so I moved it to cp-support.h.  Since there's already
>> CP_ANONYMOUS_NAMESPACE_STR/CP_ANONYMOUS_NAMESPACE_LEN there, I
>> followed the same naming pattern for the new symbols.
> 
> Looks pretty straightforward (even?) to me, but I do notice that there are still one or two places that could be updated:
> 
> c-exp.y:2281:    {"operator", OPERATOR, OP_NULL, FLAG_CXX},
> cp-name-parser.y:1839:      if (strncmp (tokstart, "operator", 8) == 0)

I had left these two because looking at the context around them, I'm
thinking that using the macro would obfuscate more than help.  c-exp.y has:

...
    {"new", NEW, OP_NULL, FLAG_CXX},
    {"delete", DELETE, OP_NULL, FLAG_CXX},
    {"operator", OPERATOR, OP_NULL, FLAG_CXX},

    {"and", ANDAND, BINOP_END, FLAG_CXX},
    {"and_eq", ASSIGN_MODIFY, BINOP_BITWISE_AND, FLAG_CXX},
    {"bitand", '&', OP_NULL, FLAG_CXX},
...

And cp-name-parser.y has:

...
    case 8:
      HANDLE_SPECIAL ("typeinfo for ", DEMANGLE_COMPONENT_TYPEINFO);
      HANDLE_SPECIAL ("typeinfo fn for ", DEMANGLE_COMPONENT_TYPEINFO_FN);
      HANDLE_SPECIAL ("typeinfo name for ", DEMANGLE_COMPONENT_TYPEINFO_NAME);
      if (strncmp (tokstart, "operator", 8) == 0)
	return OPERATOR;
      if (strncmp (tokstart, "restrict", 8) == 0)
	return RESTRICT;
      if (strncmp (tokstart, "unsigned", 8) == 0)
	return UNSIGNED;
      if (strncmp (tokstart, "template", 8) == 0)
	return TEMPLATE;
...

Note all those "8"s match the size in the switch case.  (Likewise
the other switch cases.)

Given that nowadays compilers optimize strlen("string literal") to a
constant, this cp-name-parser.y case might be tweaked as:

      if (startswith (tokstart, "operator"))
	return OPERATOR;
      if (startswith (tokstart, "restrict"))
	return RESTRICT;
      if (startswith (tokstart, "unsigned"))
	return UNSIGNED;
      if (startswith (tokstart, "template"))
	return TEMPLATE;

etc.

So for now, I've pushed the patch as it was.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index bdcd51f..24a2fbd 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1625,13 +1625,12 @@  write_destructor_name (struct parser_state *par_state, struct stoken token)
 static struct stoken
 operator_stoken (const char *op)
 {
-  static const char *operator_string = "operator";
   struct stoken st = { NULL, 0 };
   char *buf;
 
-  st.length = strlen (operator_string) + strlen (op);
+  st.length = CP_OPERATOR_LEN + strlen (op);
   buf = (char *) malloc (st.length + 1);
-  strcpy (buf, operator_string);
+  strcpy (buf, CP_OPERATOR_STR);
   strcat (buf, op);
   st.ptr = buf;
 
diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 9e197f5..890888b 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -547,7 +547,7 @@  is_type_conversion_operator (struct type *type, int i, int j)
      some other way, feel free to rewrite this function.  */
   const char *name = TYPE_FN_FIELDLIST_NAME (type, i);
 
-  if (!startswith (name, "operator"))
+  if (!startswith (name, CP_OPERATOR_STR))
     return 0;
 
   name += 8;
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 5704466..122fadd 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -931,10 +931,6 @@  cp_find_first_component (const char *name)
    the recursion easier, it also stops if it reaches an unexpected ')'
    or '>' if the value of PERMISSIVE is nonzero.  */
 
-/* Let's optimize away calls to strlen("operator").  */
-
-#define LENGTH_OF_OPERATOR 8
-
 static unsigned int
 cp_find_first_component_aux (const char *name, int permissive)
 {
@@ -1006,10 +1002,9 @@  cp_find_first_component_aux (const char *name, int permissive)
 	case 'o':
 	  /* Operator names can screw up the recursion.  */
 	  if (operator_possible
-	      && strncmp (name + index, "operator",
-			  LENGTH_OF_OPERATOR) == 0)
+	      && startswith (name + index, CP_OPERATOR_STR))
 	    {
-	      index += LENGTH_OF_OPERATOR;
+	      index += CP_OPERATOR_LEN;
 	      while (ISSPACE(name[index]))
 		++index;
 	      switch (name[index])
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 9054bf6..37b281f 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -46,6 +46,14 @@  struct using_direct;
 
 #define CP_ANONYMOUS_NAMESPACE_LEN 21
 
+/* A string representing the start of an operator name.  */
+
+#define CP_OPERATOR_STR "operator"
+
+/* The length of CP_OPERATOR_STR.  */
+
+#define CP_OPERATOR_LEN 8
+
 /* The result of parsing a name.  */
 
 struct demangle_parse_info
diff --git a/gdb/gnu-v2-abi.c b/gdb/gnu-v2-abi.c
index 0af684f..91c4201 100644
--- a/gdb/gnu-v2-abi.c
+++ b/gdb/gnu-v2-abi.c
@@ -68,7 +68,7 @@  gnuv2_is_vtable_name (const char *name)
 static int
 gnuv2_is_operator_name (const char *name)
 {
-  return startswith (name, "operator");
+  return startswith (name, CP_OPERATOR_STR);
 }
 
 
diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index 0090990..f5d3d13 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -46,7 +46,7 @@  gnuv3_is_vtable_name (const char *name)
 static int
 gnuv3_is_operator_name (const char *name)
 {
-  return startswith (name, "operator");
+  return startswith (name, CP_OPERATOR_STR);
 }
 
 
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 4c076fe..25ebdca 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -688,10 +688,9 @@  linespec_lexer_lex_string (linespec_parser *parser)
 	    {
 	      if ((PARSER_STATE (parser)->language->la_language
 		   == language_cplus)
-		  && (PARSER_STREAM (parser) - start) > 8
-		  /* strlen ("operator") */)
+		  && (PARSER_STREAM (parser) - start) > CP_OPERATOR_LEN)
 		{
-		  const char *p = strstr (start, "operator");
+		  const char *p = strstr (start, CP_OPERATOR_STR);
 
 		  if (p != NULL && is_operator_name (p))
 		    {
diff --git a/gdb/location.c b/gdb/location.c
index 8796320..d711d7b 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -24,6 +24,7 @@ 
 #include "linespec.h"
 #include "cli/cli-utils.h"
 #include "probe.h"
+#include "cp-support.h"
 
 #include <ctype.h>
 #include <string.h>
@@ -473,8 +474,8 @@  explicit_location_lex_one (const char **inp,
 	{
 	  /* Special case: C++ operator,.  */
 	  if (language->la_language == language_cplus
-	      && strncmp (*inp, "operator", 8) == 0)
-	    (*inp) += 8;
+	      && startswith (*inp, CP_OPERATOR_STR))
+	    (*inp) += CP_OPERATOR_LEN;
 	  ++(*inp);
 	}
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index cd78a16..91e8b90 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3837,9 +3837,9 @@  static const char *
 operator_chars (const char *p, const char **end)
 {
   *end = "";
-  if (!startswith (p, "operator"))
+  if (!startswith (p, CP_OPERATOR_STR))
     return *end;
-  p += 8;
+  p += CP_OPERATOR_LEN;
 
   /* Don't get faked out by `operator' being part of a longer
      identifier.  */