RFA: add C++ alternate punctuators

RFA: add C++ alternate punctuators

[Tom Tromey]

C++ has alternate spellings for some operators.  This patch adds
support for them to the C lexer.

Rather than add a bunch of new code to the switch, I followed the
comment's advice and used a table.

Built and regtested on x86-64 (compile farm).
New test case included.

Please review.

thanks,
Tom

2008-12-20  Tom Tromey  <tromey@redhat.com>

	* c-exp.y (ident_tokens): New global.
	(struct token) <cxx_only>: New field.
	(tokentab3): Update.
	(tokentab2): Update.
	(yylex): Use ident_tokens.

2008-12-20  Tom Tromey  <tromey@redhat.com>

	* gdb.cp/punctuator.exp: New file.

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 153e2be..9d32f77 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1364,36 +1364,70 @@ struct token
   char *operator;
   int token;
   enum exp_opcode opcode;
+  int cxx_only;
 };
 
 static const struct token tokentab3[] =
   {
-    {">>=", ASSIGN_MODIFY, BINOP_RSH},
-    {"<<=", ASSIGN_MODIFY, BINOP_LSH}
+    {">>=", ASSIGN_MODIFY, BINOP_RSH, 0},
+    {"<<=", ASSIGN_MODIFY, BINOP_LSH, 0}
   };
 
 static const struct token tokentab2[] =
   {
-    {"+=", ASSIGN_MODIFY, BINOP_ADD},
-    {"-=", ASSIGN_MODIFY, BINOP_SUB},
-    {"*=", ASSIGN_MODIFY, BINOP_MUL},
-    {"/=", ASSIGN_MODIFY, BINOP_DIV},
-    {"%=", ASSIGN_MODIFY, BINOP_REM},
-    {"|=", ASSIGN_MODIFY, BINOP_BITWISE_IOR},
-    {"&=", ASSIGN_MODIFY, BINOP_BITWISE_AND},
-    {"^=", ASSIGN_MODIFY, BINOP_BITWISE_XOR},
-    {"++", INCREMENT, BINOP_END},
-    {"--", DECREMENT, BINOP_END},
-    {"->", ARROW, BINOP_END},
-    {"&&", ANDAND, BINOP_END},
-    {"||", OROR, BINOP_END},
-    {"::", COLONCOLON, BINOP_END},
-    {"<<", LSH, BINOP_END},
-    {">>", RSH, BINOP_END},
-    {"==", EQUAL, BINOP_END},
-    {"!=", NOTEQUAL, BINOP_END},
-    {"<=", LEQ, BINOP_END},
-    {">=", GEQ, BINOP_END}
+    {"+=", ASSIGN_MODIFY, BINOP_ADD, 0},
+    {"-=", ASSIGN_MODIFY, BINOP_SUB, 0},
+    {"*=", ASSIGN_MODIFY, BINOP_MUL, 0},
+    {"/=", ASSIGN_MODIFY, BINOP_DIV, 0},
+    {"%=", ASSIGN_MODIFY, BINOP_REM, 0},
+    {"|=", ASSIGN_MODIFY, BINOP_BITWISE_IOR, 0},
+    {"&=", ASSIGN_MODIFY, BINOP_BITWISE_AND, 0},
+    {"^=", ASSIGN_MODIFY, BINOP_BITWISE_XOR, 0},
+    {"++", INCREMENT, BINOP_END, 0},
+    {"--", DECREMENT, BINOP_END, 0},
+    {"->", ARROW, BINOP_END, 0},
+    {"&&", ANDAND, BINOP_END, 0},
+    {"||", OROR, BINOP_END, 0},
+    {"::", COLONCOLON, BINOP_END, 0},
+    {"<<", LSH, BINOP_END, 0},
+    {">>", RSH, BINOP_END, 0},
+    {"==", EQUAL, BINOP_END, 0},
+    {"!=", NOTEQUAL, BINOP_END, 0},
+    {"<=", LEQ, BINOP_END, 0},
+    {">=", GEQ, BINOP_END, 0}
+  };
+
+/* Identifier-like tokens.  */
+static const struct token ident_tokens[] =
+  {
+    {"unsigned", UNSIGNED, OP_NULL, 0},
+    {"template", TEMPLATE, OP_NULL, 1},
+    {"volatile", VOLATILE_KEYWORD, OP_NULL, 0},
+    {"struct", STRUCT, OP_NULL, 0},
+    {"signed", SIGNED_KEYWORD, OP_NULL, 0},
+    {"sizeof", SIZEOF, OP_NULL, 0},
+    {"double", DOUBLE_KEYWORD, OP_NULL, 0},
+    {"false", FALSEKEYWORD, OP_NULL, 1},
+    {"class", CLASS, OP_NULL, 1},
+    {"union", UNION, OP_NULL, 0},
+    {"short", SHORT, OP_NULL, 0},
+    {"const", CONST_KEYWORD, OP_NULL, 0},
+    {"enum", ENUM, OP_NULL, 0},
+    {"long", LONG, OP_NULL, 0},
+    {"true", TRUEKEYWORD, OP_NULL, 1},
+    {"int", INT_KEYWORD, OP_NULL, 0},
+
+    {"and", ANDAND, BINOP_END, 1},
+    {"and_eq", ASSIGN_MODIFY, BINOP_BITWISE_AND, 1},
+    {"bitand", '&', OP_NULL, 1},
+    {"bitor", '|', OP_NULL, 1},
+    {"compl", '~', OP_NULL, 1},
+    {"not", '!', OP_NULL, 1},
+    {"not_eq", NOTEQUAL, BINOP_END, 1},
+    {"or", OROR, BINOP_END, 1},
+    {"or_eq", ASSIGN_MODIFY, BINOP_BITWISE_IOR, 1},
+    {"xor", '^', OP_NULL, 1},
+    {"xor_eq", ASSIGN_MODIFY, BINOP_BITWISE_XOR, 1}
   };
 
 /* This is set if a NAME token appeared at the very end of the input
@@ -1422,6 +1456,7 @@ yylex ()
   char * token_string = NULL;
   int class_prefix = 0;
   int saw_structop = last_was_structop;
+  char *copy;
 
   last_was_structop = 0;
 
@@ -1758,65 +1793,24 @@ yylex ()
 
   tryname:
 
-  /* Catch specific keywords.  Should be done with a data structure.  */
-  switch (namelen)
-    {
-    case 8:
-      if (strncmp (tokstart, "unsigned", 8) == 0)
-	return UNSIGNED;
-      if (parse_language->la_language == language_cplus
-	  && strncmp (tokstart, "template", 8) == 0)
-	return TEMPLATE;
-      if (strncmp (tokstart, "volatile", 8) == 0)
-	return VOLATILE_KEYWORD;
-      break;
-    case 6:
-      if (strncmp (tokstart, "struct", 6) == 0)
-	return STRUCT;
-      if (strncmp (tokstart, "signed", 6) == 0)
-	return SIGNED_KEYWORD;
-      if (strncmp (tokstart, "sizeof", 6) == 0)
-	return SIZEOF;
-      if (strncmp (tokstart, "double", 6) == 0)
-	return DOUBLE_KEYWORD;
-      break;
-    case 5:
-      if (parse_language->la_language == language_cplus)
-        {
-          if (strncmp (tokstart, "false", 5) == 0)
-            return FALSEKEYWORD;
-          if (strncmp (tokstart, "class", 5) == 0)
-            return CLASS;
-        }
-      if (strncmp (tokstart, "union", 5) == 0)
-	return UNION;
-      if (strncmp (tokstart, "short", 5) == 0)
-	return SHORT;
-      if (strncmp (tokstart, "const", 5) == 0)
-	return CONST_KEYWORD;
-      break;
-    case 4:
-      if (strncmp (tokstart, "enum", 4) == 0)
-	return ENUM;
-      if (strncmp (tokstart, "long", 4) == 0)
-	return LONG;
-      if (parse_language->la_language == language_cplus)
-          {
-            if (strncmp (tokstart, "true", 4) == 0)
-              return TRUEKEYWORD;
-          }
-      break;
-    case 3:
-      if (strncmp (tokstart, "int", 3) == 0)
-	return INT_KEYWORD;
-      break;
-    default:
-      break;
-    }
-
   yylval.sval.ptr = tokstart;
   yylval.sval.length = namelen;
 
+  /* Catch specific keywords.  */
+  copy = copy_name (yylval.sval);
+  for (i = 0; i < sizeof ident_tokens / sizeof ident_tokens[0]; i++)
+    if (strcmp (copy, ident_tokens[i].operator) == 0)
+      {
+	if (ident_tokens[i].cxx_only
+	    && parse_language->la_language != language_cplus)
+	  break;
+
+	/* It is ok to always set this, even though we don't always
+	   strictly need to.  */
+	yylval.opcode = ident_tokens[i].opcode;
+	return ident_tokens[i].token;
+      }
+
   if (*tokstart == '$')
     {
       write_dollar_variable (yylval.sval);
@@ -1829,12 +1823,11 @@ yylex ()
      currently as names of types; NAME for other symbols.
      The caller is not constrained to care about the distinction.  */
   {
-    char *tmp = copy_name (yylval.sval);
     struct symbol *sym;
     int is_a_field_of_this = 0;
     int hextype;
 
-    sym = lookup_symbol (tmp, expression_context_block,
+    sym = lookup_symbol (copy, expression_context_block,
 			 VAR_DOMAIN,
 			 parse_language->la_language == language_cplus
 			 ? &is_a_field_of_this : (int *) NULL);
@@ -1851,7 +1844,7 @@ yylex ()
       {				/* See if it's a file name. */
 	struct symtab *symtab;
 
-	symtab = lookup_symtab (tmp);
+	symtab = lookup_symtab (copy);
 
 	if (symtab)
 	  {
@@ -1870,7 +1863,7 @@ yylex ()
         }
     yylval.tsym.type
       = language_lookup_primitive_type_by_name (parse_language,
-						parse_gdbarch, tmp);
+						parse_gdbarch, copy);
     if (yylval.tsym.type != NULL)
       return TYPENAME;
 
diff --git a/gdb/testsuite/gdb.cp/punctuator.exp b/gdb/testsuite/gdb.cp/punctuator.exp
new file mode 100644
index 0000000..046f568
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/punctuator.exp
@@ -0,0 +1,52 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Simple test for alternate punctuators.
+
+# This file is part of the gdb testsuite
+
+if $tracelevel then {
+        strace $tracelevel
+        }
+
+if { [skip_cplus_tests] } { continue }
+
+gdb_exit
+gdb_start
+
+gdb_test "set lang c++" ""
+gdb_test "print (0x5a5a bitand 0xaaaa) == (0x5a5a & 0xaaaa)" " = true"
+gdb_test "print (0x5a5a bitor 0xaaaa) == (0x5a5a | 0xaaaa)" " = true"
+gdb_test "print (0x5a5a xor 0xaaaa) == (0x5a5a ^ 0xaaaa)" " = true"
+gdb_test "print (0x5a5a and 0xaaaa) == (0x5a5a && 0xaaaa)" " = true"
+gdb_test "print (0x5a5a or 0xaaaa) == (0x5a5a || 0xaaaa)" " = true"
+gdb_test "print (not not 0xaaaa) == (!!0xaaaa)" " = true"
+gdb_test "print (compl 0xaaaa) == (~0xaaaa)" " = true"
+
+gdb_test "set $u = 0x5a5a" ""
+gdb_test "set $v = 0x5a5a" ""
+gdb_test "print ($u not_eq 0xaaaa) == ($v != 0xaaaa)" "= true"
+gdb_test "print ($u and_eq 0xaaaa) == ($v &= 0xaaaa)" "= true"
+
+gdb_test "set $u = 0x5a5a" ""
+gdb_test "set $v = 0x5a5a" ""
+gdb_test "print ($u or_eq 0xaaaa) == ($v |= 0xaaaa)" "= true"
+
+gdb_test "set $u = 0x5a5a" ""
+gdb_test "set $v = 0x5a5a" ""
+gdb_test "print ($u xor_eq 0xaaaa) == ($v ^= 0xaaaa)" "= true"
+
+gdb_exit
+return 0

Re: RFA: add C++ alternate punctuators

[Pedro Alves]

On Saturday 20 December 2008 15:57:28, Tom Tromey wrote:
> C++ has alternate spellings for some operators.  This patch adds
> support for them to the C lexer.
> 
> Rather than add a bunch of new code to the switch, I followed the
> comment's advice and used a table.
> 
> Built and regtested on x86-64 (compile farm).
> New test case included.
> 
> Please review.

Looks pretty good to me.

-- 
Pedro Alves

Re: RFA: add C++ alternate punctuators

[Daniel Jacobowitz]

On Sat, Dec 20, 2008 at 08:57:28AM -0700, Tom Tromey wrote:
> C++ has alternate spellings for some operators.  This patch adds
> support for them to the C lexer.
> 
> Rather than add a bunch of new code to the switch, I followed the
> comment's advice and used a table.

Does anything performance sensitive use this lexer, or is that likely
in the future?  I suspect this version will be quite a lot slower;
it has a string copy and a linear search.

I expect the copy in cp-name-parser.c will be performance sensitive
but I don't know about this one.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFA: add C++ alternate punctuators

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> Does anything performance sensitive use this lexer, or is that likely
Daniel> in the future?  I suspect this version will be quite a lot slower;
Daniel> it has a string copy and a linear search.

I don't know whether it is performance sensitive in general.
I haven't tried profiling in this area.

The string copy is already done in the not-a-keyword case.  So, if
this part of the patch negatively affects performance, then it would
only do so for the case of expressions weighted toward use of
keywords.  That doesn't seem like a scenario worth worrying about.

We already linearly search two other tables for every token, not just
keywords.  This doesn't justify adding another one, but it does
suggest that it hasn't hurt so far.

Anyway, the above was my reasoning when I wrote the patch.  Also, I
thought using a table would make it a little less error-prone to add
new keywords in the future.  (I think we are missing 7 relevant ones.)

I can rewrite it to use a hash table if you'd prefer.  Or just inline
the new keywords into the switch and delete the comment.  Let me know.

Tom

Re: RFA: add C++ alternate punctuators

[Daniel Jacobowitz]

On Sat, Dec 20, 2008 at 10:45:11AM -0700, Tom Tromey wrote:
> Anyway, the above was my reasoning when I wrote the patch.  Also, I
> thought using a table would make it a little less error-prone to add
> new keywords in the future.  (I think we are missing 7 relevant ones.)
> 
> I can rewrite it to use a hash table if you'd prefer.  Or just inline
> the new keywords into the switch and delete the comment.  Let me know.

It's OK as you've got it.  If it becomes a problem, we can use gperf
or something.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFA: add C++ alternate punctuators

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> It's OK as you've got it.

I checked it in.  Thanks.

Tom

Re: RFA: add C++ alternate punctuators

[Jan Kratochvil]

On Mon, 22 Dec 2008 15:21:16 +0100, Tom Tromey wrote:
> I checked it in.  Thanks.

Getting:

Running ../.././gdb/testsuite/gdb.cp/punctuator.exp ...
PASS: gdb.cp/punctuator.exp: set lang c++
PASS: gdb.cp/punctuator.exp: print (0x5a5a bitand 0xaaaa) == (0x5a5a & 0xaaaa)
PASS: gdb.cp/punctuator.exp: print (0x5a5a bitor 0xaaaa) == (0x5a5a | 0xaaaa)
PASS: gdb.cp/punctuator.exp: print (0x5a5a xor 0xaaaa) == (0x5a5a ^ 0xaaaa)
PASS: gdb.cp/punctuator.exp: print (0x5a5a and 0xaaaa) == (0x5a5a && 0xaaaa)
PASS: gdb.cp/punctuator.exp: print (0x5a5a or 0xaaaa) == (0x5a5a || 0xaaaa)
PASS: gdb.cp/punctuator.exp: print (not not 0xaaaa) == (!!0xaaaa)
PASS: gdb.cp/punctuator.exp: print (compl 0xaaaa) == (~0xaaaa)
ERROR: tcl error sourcing ../.././gdb/testsuite/gdb.cp/punctuator.exp.
ERROR: can't read "u": no such variable
    while executing
"gdb_test "set $u = 0x5a5a" """
    (file "../.././gdb/testsuite/gdb.cp/punctuator.exp" line 38)
    invoked from within
"source ../.././gdb/testsuite/gdb.cp/punctuator.exp"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 source ../.././gdb/testsuite/gdb.cp/punctuator.exp"
    invoked from within
"catch "uplevel #0 source $test_file_name""


on dejagnu-1.4.4-13.fc10.noarch, I do not understand where it could work.

OK to check in?


Regards,
Jan


gdb/testsuite/
2008-12-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.cp/punctuator.exp: Backslash the '$' signs.

--- gdb/testsuite/gdb.cp/punctuator.exp	22 Dec 2008 14:21:01 -0000	1.1
+++ gdb/testsuite/gdb.cp/punctuator.exp	23 Dec 2008 09:58:46 -0000
@@ -35,18 +35,18 @@ gdb_test "print (0x5a5a or 0xaaaa) == (0
 gdb_test "print (not not 0xaaaa) == (!!0xaaaa)" " = true"
 gdb_test "print (compl 0xaaaa) == (~0xaaaa)" " = true"
 
-gdb_test "set $u = 0x5a5a" ""
-gdb_test "set $v = 0x5a5a" ""
-gdb_test "print ($u not_eq 0xaaaa) == ($v != 0xaaaa)" "= true"
-gdb_test "print ($u and_eq 0xaaaa) == ($v &= 0xaaaa)" "= true"
+gdb_test "set \$u = 0x5a5a" ""
+gdb_test "set \$v = 0x5a5a" ""
+gdb_test "print (\$u not_eq 0xaaaa) == (\$v != 0xaaaa)" "= true"
+gdb_test "print (\$u and_eq 0xaaaa) == (\$v &= 0xaaaa)" "= true"
 
-gdb_test "set $u = 0x5a5a" ""
-gdb_test "set $v = 0x5a5a" ""
-gdb_test "print ($u or_eq 0xaaaa) == ($v |= 0xaaaa)" "= true"
+gdb_test "set \$u = 0x5a5a" ""
+gdb_test "set \$v = 0x5a5a" ""
+gdb_test "print (\$u or_eq 0xaaaa) == (\$v |= 0xaaaa)" "= true"
 
-gdb_test "set $u = 0x5a5a" ""
-gdb_test "set $v = 0x5a5a" ""
-gdb_test "print ($u xor_eq 0xaaaa) == ($v ^= 0xaaaa)" "= true"
+gdb_test "set \$u = 0x5a5a" ""
+gdb_test "set \$v = 0x5a5a" ""
+gdb_test "print (\$u xor_eq 0xaaaa) == (\$v ^= 0xaaaa)" "= true"
 
 gdb_exit
 return 0

Re: RFA: add C++ alternate punctuators

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> ERROR: can't read "u": no such variable
Jan>     while executing
Jan> "gdb_test "set $u = 0x5a5a" """
Jan>     (file "../.././gdb/testsuite/gdb.cp/punctuator.exp" line 38)
Jan>     invoked from within
Jan> "source ../.././gdb/testsuite/gdb.cp/punctuator.exp"
Jan>     ("uplevel" body line 1)
Jan>     invoked from within
Jan> "uplevel #0 source ../.././gdb/testsuite/gdb.cp/punctuator.exp"
Jan>     invoked from within
Jan> "catch "uplevel #0 source $test_file_name""

Jan> on dejagnu-1.4.4-13.fc10.noarch, I do not understand where it could work.

Me neither!  WTF?  I ran this many times and didn't see it.

But, of course, I do see it now.  How embarrassing.

Jan> 	* gdb.cp/punctuator.exp: Backslash the '$' signs.

IMO this ought to qualify under the obvious rule.

Tom

Re: RFA: add C++ alternate punctuators

[Joel Brobecker]

> gdb/testsuite/
> 2008-12-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.cp/punctuator.exp: Backslash the '$' signs.


Yes, please! As Tom said, any other such changes qualify for obvious,
particularly when they fix a failure causing a testsuite abort.

Thank you.

-- 
Joel

Re: RFA: add C++ alternate punctuators

[Jan Kratochvil]

On Tue, 23 Dec 2008 17:36:13 +0100, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> ERROR: can't read "u": no such variable
...
> But, of course, I do see it now.  How embarrassing.

Checked in.

> IMO this ought to qualify under the obvious rule.
On Tue, 23 Dec 2008 18:01:42 +0100, Joel Brobecker wrote:
> Yes, please! As Tom said, any other such changes qualify for obvious,

I was more curious how could such typo happen so I was suspecting if I do not
have some incompatible(/old(?)) tcl (tcl, not dejagnu) so I rather asked.


Regards,
Jan