[PATCH v2] Do not classify C struct members as a filename

[PATCH v2] Do not classify C struct members as a filename

[Leszek Swirski via gdb-patches]

There is existing logic in C/C++ expression parsing to avoid classifying
names as a filename when they are a field on the this object. This
change extends this logic to also avoid classifying names after a
struct-op (-> or .) as a filename, which otherwise causes a syntax
error.

Thus, it is now possible in the file

    #include <map>
    struct D {
        void map();
    }
    D d;

to call

    (gdb) print d.map()

where previously this would have been a syntax error.

Tested on gdb.cp/*.exp

gdb/ChangeLog:

        * c-exp.y (lex_one_token, classify_name, yylex): Don't classify
        names after a structop as a filename

gdb/testsuite/ChangeLog:

        * gdb.cp/filename.cc, gdb.cp/filename.exp: Test that member
        functions with the same name as an include file are parsed
        correctly.
---

Removed an accidental unrelated change in varobj.c

 gdb/ChangeLog                     |  5 +++++
 gdb/c-exp.y                       | 24 +++++++++++++-----------
 gdb/testsuite/ChangeLog           |  6 ++++++
 gdb/testsuite/gdb.cp/filename.cc  | 15 ++++++++++++++-
 gdb/testsuite/gdb.cp/filename.exp | 10 ++++++++--
 5 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2b6d1d6d6b..17a5a84b0c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-01-24  Leszek Swirski  <leszeks@google.com>
+
+	* c-exp.y (lex_one_token, classify_name, yylex): Don't classify
+	names after a structop as a filename
+
 2018-01-23  Philipp Rudo  <prudo@linux.vnet.ibm.com>
 
 	* s390-linux-tdep.c (s390_record_address_mask)
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 0482e85ce8..f8c5529551 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -2447,8 +2447,7 @@ static struct macro_scope *expression_macro_scope;
 static int saw_name_at_eof;
 
 /* This is set if the previously-returned token was a structure
-   operator -- either '.' or ARROW.  This is used only when parsing to
-   do field name completion.  */
+   operator -- either '.' or ARROW.  */
 static int last_was_structop;
 
 /* Read one token, getting characters through lexptr.  */
@@ -2505,7 +2504,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
 
 	lexptr += 2;
 	yylval.opcode = tokentab2[i].opcode;
-	if (parse_completion && tokentab2[i].token == ARROW)
+	if (tokentab2[i].token == ARROW)
 	  last_was_structop = 1;
 	return tokentab2[i].token;
       }
@@ -2569,8 +2568,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
       /* Might be a floating point number.  */
       if (lexptr[1] < '0' || lexptr[1] > '9')
 	{
-	  if (parse_completion)
-	    last_was_structop = 1;
+	  last_was_structop = 1;
 	  goto symbol;		/* Nope, must be a symbol. */
 	}
       /* FALL THRU into number case.  */
@@ -2863,7 +2861,7 @@ auto_obstack name_obstack;
 
 static int
 classify_name (struct parser_state *par_state, const struct block *block,
-	       int is_quoted_name)
+	       int is_quoted_name, int is_after_structop)
 {
   struct block_symbol bsym;
   char *copy;
@@ -2907,11 +2905,13 @@ classify_name (struct parser_state *par_state, const struct block *block,
 	    }
 	}
 
-      /* If we found a field, then we want to prefer it over a
+      /* If we found a field on the "this" object, or we are looking
+	 up a field on a struct, then we want to prefer it over a
 	 filename.  However, if the name was quoted, then it is better
 	 to check for a filename or a block, since this is the only
 	 way the user has of requiring the extension to be used.  */
-      if (is_a_field_of_this.type == NULL || is_quoted_name)
+      if ((is_a_field_of_this.type == NULL && !is_after_structop) 
+	  || is_quoted_name)
 	{
 	  /* See if it's a file name. */
 	  struct symtab *symtab;
@@ -2992,7 +2992,7 @@ classify_inner_name (struct parser_state *par_state,
   char *copy;
 
   if (context == NULL)
-    return classify_name (par_state, block, 0);
+    return classify_name (par_state, block, 0, 0);
 
   type = check_typedef (context);
   if (!type_aggregate_p (type))
@@ -3062,7 +3062,7 @@ static int
 yylex (void)
 {
   token_and_value current;
-  int first_was_coloncolon, last_was_coloncolon;
+  int first_was_coloncolon, last_was_coloncolon, last_lex_was_structop;
   struct type *context_type = NULL;
   int last_to_examine, next_to_examine, checkpoint;
   const struct block *search_block;
@@ -3072,13 +3072,15 @@ yylex (void)
     goto do_pop;
   popping = 0;
 
+  last_lex_was_structop = last_was_structop;
+
   /* Read the first token and decide what to do.  Most of the
      subsequent code is C++-only; but also depends on seeing a "::" or
      name-like token.  */
   current.token = lex_one_token (pstate, &is_quoted_name);
   if (current.token == NAME)
     current.token = classify_name (pstate, expression_context_block,
-				   is_quoted_name);
+				   is_quoted_name, last_lex_was_structop);
   if (parse_language (pstate)->la_language != language_cplus
       || (current.token != TYPENAME && current.token != COLONCOLON
 	  && current.token != FILENAME))
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0f02f4a97f..903d9f4cb6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2018-01-24  Leszek Swirski  <leszeks@google.com>
+
+	* gdb.cp/filename.cc, gdb.cp/filename.exp: Test that member
+	functions with the same name as an include file are parsed
+	correctly.
+
 2018-01-22  Pedro Alves  <palves@redhat.com>
 	    Sergio Durigan Junior  <sergiodj@redhat.com>
 
diff --git a/gdb/testsuite/gdb.cp/filename.cc b/gdb/testsuite/gdb.cp/filename.cc
index 45edf4efe9..0600c67a75 100644
--- a/gdb/testsuite/gdb.cp/filename.cc
+++ b/gdb/testsuite/gdb.cp/filename.cc
@@ -26,11 +26,24 @@ public:
   }
 
   void m() {
-    /* stop here */
+    /* stop here 1 */
   }
 };
 
+class D {
+public:
+  int includefile();
+};
+
+int D::includefile() {
+  return 24;
+}
+
 int main() {
   C c;
   c.m();
+
+  D d;
+  D* pd = &d;
+  /* stop here 2 */
 }
diff --git a/gdb/testsuite/gdb.cp/filename.exp b/gdb/testsuite/gdb.cp/filename.exp
index 971ffe715f..67583cc67c 100644
--- a/gdb/testsuite/gdb.cp/filename.exp
+++ b/gdb/testsuite/gdb.cp/filename.exp
@@ -26,8 +26,14 @@ if ![runto_main] then {
     continue
 }
 
-gdb_breakpoint [gdb_get_line_number "stop here"]
-gdb_continue_to_breakpoint "stop here"
+gdb_breakpoint [gdb_get_line_number "stop here 1"]
+gdb_continue_to_breakpoint "stop here 1"
 
 gdb_test "print includefile\[0\]" " = 23"
 gdb_test "print 'includefile'::some_global" " = 27"
+
+gdb_breakpoint [gdb_get_line_number "stop here 2"]
+gdb_continue_to_breakpoint "stop here 2"
+
+gdb_test "print d.includefile()" " = 24"
+gdb_test "print pd->includefile()" " = 24"
-- 
2.16.0.rc1.238.g530d649a79-goog

Re: [PATCH v2] Do not classify C struct members as a filename

[Simon Marchi]

On 2018-01-24 11:51, Leszek Swirski via gdb-patches wrote:
> There is existing logic in C/C++ expression parsing to avoid 
> classifying
> names as a filename when they are a field on the this object. This
> change extends this logic to also avoid classifying names after a
> struct-op (-> or .) as a filename, which otherwise causes a syntax
> error.
> 
> Thus, it is now possible in the file
> 
>     #include <map>
>     struct D {
>         void map();
>     }
>     D d;
> 
> to call
> 
>     (gdb) print d.map()
> 
> where previously this would have been a syntax error.
> 
> Tested on gdb.cp/*.exp

Hi Leszek,

I was able to reproduce the problem and confirmed that the patch fixes 
it.  I'm not very strong in the lexing/parsing area, but the change in 
c-exp.y makes sense to me.  I'd like to give others a chance to comment, 
so let's wait a few days.  If nobody answer, we can merge it.  One nit, 
since we are now using C++, can you use bool instead of int?  It won't 
match the old surrounding code, but that's a good reason to modernize 
the existing code later :).

Do you know if we have a test for the same thing, but with the "this" 
pointer (which was already worked before this patch)?  If not, it would 
be nice to add it to the test as well.  You could add a call to 
D::includefile and continue/break there.  From there, you could test 
calling this->includefile().

Simon

Re: [PATCH v2] Do not classify C struct members as a filename

[Leszek Swirski via gdb-patches]

>
> I was able to reproduce the problem and confirmed that the patch fixes
> it.  I'm not very strong in the lexing/parsing area, but the change in
> c-exp.y makes sense to me.  I'd like to give others a chance to comment, so
> let's wait a few days.  If nobody answer, we can merge it.  One nit, since
> we are now using C++, can you use bool instead of int?  It won't match the
> old surrounding code, but that's a good reason to modernize the existing
> code later :).
>
> Do you know if we have a test for the same thing, but with the "this"
> pointer (which was already worked before this patch)?  If not, it would be
> nice to add it to the test as well.  You could add a call to D::includefile
> and continue/break there.  From there, you could test calling
> this->includefile().


Hi Simon, thanks for taking a look.

Sure, I can update the tests (I'll add some tests for checking
"c.includefile" while I'm at it) and change the int to a bool. I'm not 100%
sure on the procedure here, do you want me to upload a v3 as a new thread?

- Leszek

Re: [PATCH v2] Do not classify C struct members as a filename

[Simon Marchi]

On 2018-01-25 05:03, Leszek Swirski via gdb-patches wrote:
> Hi Simon, thanks for taking a look.
> 
> Sure, I can update the tests (I'll add some tests for checking
> "c.includefile" while I'm at it) and change the int to a bool. I'm not 
> 100%
> sure on the procedure here, do you want me to upload a v3 as a new 
> thread?
> 
> - Leszek

Hi Leszek,

Yes, sending a v3 would be ideal.  It's usually a new thread (as in 
there is no "Re:"), but some people like to use git-send-email's 
--in-reply-to option to refer to the previous version, so that the 
different versions thread nicely in email clients.  It's up to you.

Simon

[PATCH v3] Do not classify C struct members as a filename

[Leszek Swirski via gdb-patches]

There is existing logic in C/C++ expression parsing to avoid classifying
names as a filename when they are a field on the this object. This
change extends this logic to also avoid classifying names after a
struct-op (-> or .) as a filename, which otherwise causes a syntax
error.

Thus, it is now possible in the file

    #include <map>
    struct D {
        void map();
    }
    D d;

to call

    (gdb) print d.map()

where previously this would have been a syntax error.

Tested on gdb.cp/*.exp

gdb/ChangeLog:

        * c-exp.y (lex_one_token, classify_name, yylex): Don't classify
        names after a structop as a filename

gdb/testsuite/ChangeLog:

        * gdb.cp/filename.cc, gdb.cp/filename.exp: Test that member
        functions with the same name as an include file are parsed
        correctly.
---

Updated ints to bools (including some opportunistic updates of existing
arguments), and expanded test.

 gdb/ChangeLog                     |  5 +++++
 gdb/c-exp.y                       | 43 +++++++++++++++++++++------------------
 gdb/testsuite/ChangeLog           |  6 ++++++
 gdb/testsuite/gdb.cp/filename.cc  | 22 +++++++++++++++++++-
 gdb/testsuite/gdb.cp/filename.exp | 20 ++++++++++++++++--
 5 files changed, 73 insertions(+), 23 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2b6d1d6d6b..17a5a84b0c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-01-24  Leszek Swirski  <leszeks@google.com>
+
+	* c-exp.y (lex_one_token, classify_name, yylex): Don't classify
+	names after a structop as a filename
+
 2018-01-23  Philipp Rudo  <prudo@linux.vnet.ibm.com>
 
 	* s390-linux-tdep.c (s390_record_address_mask)
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 0482e85ce8..5802a6d746 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -2447,24 +2447,23 @@ static struct macro_scope *expression_macro_scope;
 static int saw_name_at_eof;
 
 /* This is set if the previously-returned token was a structure
-   operator -- either '.' or ARROW.  This is used only when parsing to
-   do field name completion.  */
-static int last_was_structop;
+   operator -- either '.' or ARROW.  */
+static bool last_was_structop;
 
 /* Read one token, getting characters through lexptr.  */
 
 static int
-lex_one_token (struct parser_state *par_state, int *is_quoted_name)
+lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
 {
   int c;
   int namelen;
   unsigned int i;
   const char *tokstart;
-  int saw_structop = last_was_structop;
+  bool saw_structop = last_was_structop;
   char *copy;
 
-  last_was_structop = 0;
-  *is_quoted_name = 0;
+  last_was_structop = false;
+  *is_quoted_name = false;
 
  retry:
 
@@ -2505,7 +2504,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
 
 	lexptr += 2;
 	yylval.opcode = tokentab2[i].opcode;
-	if (parse_completion && tokentab2[i].token == ARROW)
+	if (tokentab2[i].token == ARROW)
 	  last_was_structop = 1;
 	return tokentab2[i].token;
       }
@@ -2569,8 +2568,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
       /* Might be a floating point number.  */
       if (lexptr[1] < '0' || lexptr[1] > '9')
 	{
-	  if (parse_completion)
-	    last_was_structop = 1;
+	  last_was_structop = true;
 	  goto symbol;		/* Nope, must be a symbol. */
 	}
       /* FALL THRU into number case.  */
@@ -2711,7 +2709,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
 	      {
 		++tokstart;
 		namelen = lexptr - tokstart - 1;
-		*is_quoted_name = 1;
+		*is_quoted_name = true;
 
 		goto tryname;
 	      }
@@ -2859,11 +2857,12 @@ auto_obstack name_obstack;
    Updates yylval and returns the new token type.  BLOCK is the block
    in which lookups start; this can be NULL to mean the global scope.
    IS_QUOTED_NAME is non-zero if the name token was originally quoted
-   in single quotes.  */
+   in single quotes.  IS_AFTER_STRUCTOP is true if this name follows
+   a structure operator -- either '.' or ARROW  */
 
 static int
 classify_name (struct parser_state *par_state, const struct block *block,
-	       int is_quoted_name)
+	       bool is_quoted_name, bool is_after_structop)
 {
   struct block_symbol bsym;
   char *copy;
@@ -2907,11 +2906,13 @@ classify_name (struct parser_state *par_state, const struct block *block,
 	    }
 	}
 
-      /* If we found a field, then we want to prefer it over a
+      /* If we found a field on the "this" object, or we are looking
+	 up a field on a struct, then we want to prefer it over a
 	 filename.  However, if the name was quoted, then it is better
 	 to check for a filename or a block, since this is the only
 	 way the user has of requiring the extension to be used.  */
-      if (is_a_field_of_this.type == NULL || is_quoted_name)
+      if ((is_a_field_of_this.type == NULL && !is_after_structop) 
+	  || is_quoted_name)
 	{
 	  /* See if it's a file name. */
 	  struct symtab *symtab;
@@ -2992,7 +2993,7 @@ classify_inner_name (struct parser_state *par_state,
   char *copy;
 
   if (context == NULL)
-    return classify_name (par_state, block, 0);
+    return classify_name (par_state, block, false, false);
 
   type = check_typedef (context);
   if (!type_aggregate_p (type))
@@ -3066,19 +3067,21 @@ yylex (void)
   struct type *context_type = NULL;
   int last_to_examine, next_to_examine, checkpoint;
   const struct block *search_block;
-  int is_quoted_name;
+  bool is_quoted_name, last_lex_was_structop;
 
   if (popping && !VEC_empty (token_and_value, token_fifo))
     goto do_pop;
   popping = 0;
 
+  last_lex_was_structop = last_was_structop;
+
   /* Read the first token and decide what to do.  Most of the
      subsequent code is C++-only; but also depends on seeing a "::" or
      name-like token.  */
   current.token = lex_one_token (pstate, &is_quoted_name);
   if (current.token == NAME)
     current.token = classify_name (pstate, expression_context_block,
-				   is_quoted_name);
+				   is_quoted_name, last_lex_was_structop);
   if (parse_language (pstate)->la_language != language_cplus
       || (current.token != TYPENAME && current.token != COLONCOLON
 	  && current.token != FILENAME))
@@ -3091,7 +3094,7 @@ yylex (void)
   last_was_coloncolon = current.token == COLONCOLON;
   while (1)
     {
-      int ignore;
+      bool ignore;
 
       /* We ignore quoted names other than the very first one.
 	 Subsequent ones do not have any special meaning.  */
@@ -3242,7 +3245,7 @@ c_parse (struct parser_state *par_state)
 							parser_debug);
 
   /* Initialize some state used by the lexer.  */
-  last_was_structop = 0;
+  last_was_structop = false;
   saw_name_at_eof = 0;
 
   VEC_free (token_and_value, token_fifo);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0f02f4a97f..903d9f4cb6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2018-01-24  Leszek Swirski  <leszeks@google.com>
+
+	* gdb.cp/filename.cc, gdb.cp/filename.exp: Test that member
+	functions with the same name as an include file are parsed
+	correctly.
+
 2018-01-22  Pedro Alves  <palves@redhat.com>
 	    Sergio Durigan Junior  <sergiodj@redhat.com>
 
diff --git a/gdb/testsuite/gdb.cp/filename.cc b/gdb/testsuite/gdb.cp/filename.cc
index 45edf4efe9..d33ef78bcb 100644
--- a/gdb/testsuite/gdb.cp/filename.cc
+++ b/gdb/testsuite/gdb.cp/filename.cc
@@ -26,11 +26,31 @@ public:
   }
 
   void m() {
-    /* stop here */
+    /* stop inside C */
   }
 };
 
+class D {
+public:
+  int includefile();
+
+  void m() {
+    /* stop inside D */
+  }
+};
+
+int D::includefile() {
+  return 24;
+}
+
 int main() {
   C c;
+  C* pc = &c;
   c.m();
+
+  D d;
+  D* pd = &d;
+  d.m();
+
+  /* stop outside */
 }
diff --git a/gdb/testsuite/gdb.cp/filename.exp b/gdb/testsuite/gdb.cp/filename.exp
index 971ffe715f..208179e7ca 100644
--- a/gdb/testsuite/gdb.cp/filename.exp
+++ b/gdb/testsuite/gdb.cp/filename.exp
@@ -26,8 +26,24 @@ if ![runto_main] then {
     continue
 }
 
-gdb_breakpoint [gdb_get_line_number "stop here"]
-gdb_continue_to_breakpoint "stop here"
+gdb_breakpoint [gdb_get_line_number "stop inside C"]
+gdb_continue_to_breakpoint "stop inside C"
 
 gdb_test "print includefile\[0\]" " = 23"
+gdb_test "print this->includefile\[0\]" " = 23"
 gdb_test "print 'includefile'::some_global" " = 27"
+
+gdb_breakpoint [gdb_get_line_number "stop inside D"]
+gdb_continue_to_breakpoint "stop inside D"
+
+gdb_test "print includefile()" " = 24"
+gdb_test "print this->includefile()" " = 24"
+gdb_test "print 'includefile'::some_global" " = 27"
+
+gdb_breakpoint [gdb_get_line_number "stop outside"]
+gdb_continue_to_breakpoint "stop outside"
+
+gdb_test "print c.includefile\[0\]" " = 23"
+gdb_test "print pc->includefile\[0\]" " = 23"
+gdb_test "print d.includefile()" " = 24"
+gdb_test "print pd->includefile()" " = 24"
-- 
2.16.0.rc1.238.g530d649a79-goog

Re: [PATCH v2] Do not classify C struct members as a filename

[Tom Tromey]

>>>>> "Leszek" == Leszek Swirski via gdb-patches <gdb-patches@sourceware.org> writes:

Leszek> There is existing logic in C/C++ expression parsing to avoid classifying
Leszek> names as a filename when they are a field on the this object. This
Leszek> change extends this logic to also avoid classifying names after a
Leszek> struct-op (-> or .) as a filename, which otherwise causes a syntax
Leszek> error.

Nice, thanks for doing this.

Leszek> -	if (parse_completion && tokentab2[i].token == ARROW)
Leszek> +	if (tokentab2[i].token == ARROW)

Leszek> -	  if (parse_completion)
Leszek> -	    last_was_structop = 1;
Leszek> +	  last_was_structop = 1;

This change makes sense (and thanks for updating that comment as well),
but I wonder whether this changes the behavior in some case.  Elsewhere
in lex_one_token there is a check of saw_structop:

      else if (saw_structop)
	return COMPLETE;

Previously this return could only be taken if parse_completion was set,
but now I think it could be taken in other situations.

So, I suspect "parse_completion &&" should be stuck in there.

Tom

[PATCH v4] Do not classify C struct members as a filename

[Leszek Swirski via gdb-patches]

There is existing logic in C/C++ expression parsing to avoid classifying
names as a filename when they are a field on the this object. This
change extends this logic to also avoid classifying names after a
struct-op (-> or .) as a filename, which otherwise causes a syntax
error.

Thus, it is now possible in the file

    #include <map>
    struct D {
        void map();
    }
    D d;

to call

    (gdb) print d.map()

where previously this would have been a syntax error.

Tested on gdb.cp/*.exp

gdb/ChangeLog:

        * c-exp.y (lex_one_token, classify_name, yylex): Don't classify
        names after a structop as a filename

gdb/testsuite/ChangeLog:

        * gdb.cp/filename.cc, gdb.cp/filename.exp: Test that member
        functions with the same name as an include file are parsed
        correctly.
---

Fix completion token to only appear on completion.

 gdb/ChangeLog                     |  5 +++++
 gdb/c-exp.y                       | 45 +++++++++++++++++++++------------------
 gdb/testsuite/ChangeLog           |  6 ++++++
 gdb/testsuite/gdb.cp/filename.cc  | 22 ++++++++++++++++++-
 gdb/testsuite/gdb.cp/filename.exp | 20 +++++++++++++++--
 5 files changed, 74 insertions(+), 24 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2b6d1d6d6b..17a5a84b0c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-01-24  Leszek Swirski  <leszeks@google.com>
+
+	* c-exp.y (lex_one_token, classify_name, yylex): Don't classify
+	names after a structop as a filename
+
 2018-01-23  Philipp Rudo  <prudo@linux.vnet.ibm.com>
 
 	* s390-linux-tdep.c (s390_record_address_mask)
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 0482e85ce8..8f0aa00f52 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -2447,24 +2447,23 @@ static struct macro_scope *expression_macro_scope;
 static int saw_name_at_eof;
 
 /* This is set if the previously-returned token was a structure
-   operator -- either '.' or ARROW.  This is used only when parsing to
-   do field name completion.  */
-static int last_was_structop;
+   operator -- either '.' or ARROW.  */
+static bool last_was_structop;
 
 /* Read one token, getting characters through lexptr.  */
 
 static int
-lex_one_token (struct parser_state *par_state, int *is_quoted_name)
+lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
 {
   int c;
   int namelen;
   unsigned int i;
   const char *tokstart;
-  int saw_structop = last_was_structop;
+  bool saw_structop = last_was_structop;
   char *copy;
 
-  last_was_structop = 0;
-  *is_quoted_name = 0;
+  last_was_structop = false;
+  *is_quoted_name = false;
 
  retry:
 
@@ -2505,7 +2504,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
 
 	lexptr += 2;
 	yylval.opcode = tokentab2[i].opcode;
-	if (parse_completion && tokentab2[i].token == ARROW)
+	if (tokentab2[i].token == ARROW)
 	  last_was_structop = 1;
 	return tokentab2[i].token;
       }
@@ -2529,7 +2528,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
 	  saw_name_at_eof = 0;
 	  return COMPLETE;
 	}
-      else if (saw_structop)
+      else if (parse_completion && saw_structop)
 	return COMPLETE;
       else
         return 0;
@@ -2569,8 +2568,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
       /* Might be a floating point number.  */
       if (lexptr[1] < '0' || lexptr[1] > '9')
 	{
-	  if (parse_completion)
-	    last_was_structop = 1;
+	  last_was_structop = true;
 	  goto symbol;		/* Nope, must be a symbol. */
 	}
       /* FALL THRU into number case.  */
@@ -2711,7 +2709,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
 	      {
 		++tokstart;
 		namelen = lexptr - tokstart - 1;
-		*is_quoted_name = 1;
+		*is_quoted_name = true;
 
 		goto tryname;
 	      }
@@ -2859,11 +2857,12 @@ auto_obstack name_obstack;
    Updates yylval and returns the new token type.  BLOCK is the block
    in which lookups start; this can be NULL to mean the global scope.
    IS_QUOTED_NAME is non-zero if the name token was originally quoted
-   in single quotes.  */
+   in single quotes.  IS_AFTER_STRUCTOP is true if this name follows
+   a structure operator -- either '.' or ARROW  */
 
 static int
 classify_name (struct parser_state *par_state, const struct block *block,
-	       int is_quoted_name)
+	       bool is_quoted_name, bool is_after_structop)
 {
   struct block_symbol bsym;
   char *copy;
@@ -2907,11 +2906,13 @@ classify_name (struct parser_state *par_state, const struct block *block,
 	    }
 	}
 
-      /* If we found a field, then we want to prefer it over a
+      /* If we found a field on the "this" object, or we are looking
+	 up a field on a struct, then we want to prefer it over a
 	 filename.  However, if the name was quoted, then it is better
 	 to check for a filename or a block, since this is the only
 	 way the user has of requiring the extension to be used.  */
-      if (is_a_field_of_this.type == NULL || is_quoted_name)
+      if ((is_a_field_of_this.type == NULL && !is_after_structop) 
+	  || is_quoted_name)
 	{
 	  /* See if it's a file name. */
 	  struct symtab *symtab;
@@ -2992,7 +2993,7 @@ classify_inner_name (struct parser_state *par_state,
   char *copy;
 
   if (context == NULL)
-    return classify_name (par_state, block, 0);
+    return classify_name (par_state, block, false, false);
 
   type = check_typedef (context);
   if (!type_aggregate_p (type))
@@ -3066,19 +3067,21 @@ yylex (void)
   struct type *context_type = NULL;
   int last_to_examine, next_to_examine, checkpoint;
   const struct block *search_block;
-  int is_quoted_name;
+  bool is_quoted_name, last_lex_was_structop;
 
   if (popping && !VEC_empty (token_and_value, token_fifo))
     goto do_pop;
   popping = 0;
 
+  last_lex_was_structop = last_was_structop;
+
   /* Read the first token and decide what to do.  Most of the
      subsequent code is C++-only; but also depends on seeing a "::" or
      name-like token.  */
   current.token = lex_one_token (pstate, &is_quoted_name);
   if (current.token == NAME)
     current.token = classify_name (pstate, expression_context_block,
-				   is_quoted_name);
+				   is_quoted_name, last_lex_was_structop);
   if (parse_language (pstate)->la_language != language_cplus
       || (current.token != TYPENAME && current.token != COLONCOLON
 	  && current.token != FILENAME))
@@ -3091,7 +3094,7 @@ yylex (void)
   last_was_coloncolon = current.token == COLONCOLON;
   while (1)
     {
-      int ignore;
+      bool ignore;
 
       /* We ignore quoted names other than the very first one.
 	 Subsequent ones do not have any special meaning.  */
@@ -3242,7 +3245,7 @@ c_parse (struct parser_state *par_state)
 							parser_debug);
 
   /* Initialize some state used by the lexer.  */
-  last_was_structop = 0;
+  last_was_structop = false;
   saw_name_at_eof = 0;
 
   VEC_free (token_and_value, token_fifo);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0f02f4a97f..903d9f4cb6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2018-01-24  Leszek Swirski  <leszeks@google.com>
+
+	* gdb.cp/filename.cc, gdb.cp/filename.exp: Test that member
+	functions with the same name as an include file are parsed
+	correctly.
+
 2018-01-22  Pedro Alves  <palves@redhat.com>
 	    Sergio Durigan Junior  <sergiodj@redhat.com>
 
diff --git a/gdb/testsuite/gdb.cp/filename.cc b/gdb/testsuite/gdb.cp/filename.cc
index 45edf4efe9..d33ef78bcb 100644
--- a/gdb/testsuite/gdb.cp/filename.cc
+++ b/gdb/testsuite/gdb.cp/filename.cc
@@ -26,11 +26,31 @@ public:
   }
 
   void m() {
-    /* stop here */
+    /* stop inside C */
   }
 };
 
+class D {
+public:
+  int includefile();
+
+  void m() {
+    /* stop inside D */
+  }
+};
+
+int D::includefile() {
+  return 24;
+}
+
 int main() {
   C c;
+  C* pc = &c;
   c.m();
+
+  D d;
+  D* pd = &d;
+  d.m();
+
+  /* stop outside */
 }
diff --git a/gdb/testsuite/gdb.cp/filename.exp b/gdb/testsuite/gdb.cp/filename.exp
index 971ffe715f..208179e7ca 100644
--- a/gdb/testsuite/gdb.cp/filename.exp
+++ b/gdb/testsuite/gdb.cp/filename.exp
@@ -26,8 +26,24 @@ if ![runto_main] then {
     continue
 }
 
-gdb_breakpoint [gdb_get_line_number "stop here"]
-gdb_continue_to_breakpoint "stop here"
+gdb_breakpoint [gdb_get_line_number "stop inside C"]
+gdb_continue_to_breakpoint "stop inside C"
 
 gdb_test "print includefile\[0\]" " = 23"
+gdb_test "print this->includefile\[0\]" " = 23"
 gdb_test "print 'includefile'::some_global" " = 27"
+
+gdb_breakpoint [gdb_get_line_number "stop inside D"]
+gdb_continue_to_breakpoint "stop inside D"
+
+gdb_test "print includefile()" " = 24"
+gdb_test "print this->includefile()" " = 24"
+gdb_test "print 'includefile'::some_global" " = 27"
+
+gdb_breakpoint [gdb_get_line_number "stop outside"]
+gdb_continue_to_breakpoint "stop outside"
+
+gdb_test "print c.includefile\[0\]" " = 23"
+gdb_test "print pc->includefile\[0\]" " = 23"
+gdb_test "print d.includefile()" " = 24"
+gdb_test "print pd->includefile()" " = 24"
-- 
2.16.0.rc1.238.g530d649a79-goog

Re: [PATCH v2] Do not classify C struct members as a filename

[Leszek Swirski via gdb-patches]

> This change makes sense (and thanks for updating that comment as well),
> but I wonder whether this changes the behavior in some case.  Elsewhere
> in lex_one_token there is a check of saw_structop:
>
>       else if (saw_structop)
>         return COMPLETE;
>
> Previously this return could only be taken if parse_completion was set,
> but now I think it could be taken in other situations.
>
> So, I suspect "parse_completion &&" should be stuck in there.

Nice catch, thanks! I looked at the other users of saw_structop and
last_was_structop, and it looks like this is the only location that
needs an update. PTAL at v4.

Re: [PATCH v4] Do not classify C struct members as a filename

[Simon Marchi]

On 2018-01-25 11:20 AM, Leszek Swirski wrote:
> There is existing logic in C/C++ expression parsing to avoid classifying
> names as a filename when they are a field on the this object. This
> change extends this logic to also avoid classifying names after a
> struct-op (-> or .) as a filename, which otherwise causes a syntax
> error.
> 
> Thus, it is now possible in the file
> 
>     #include <map>
>     struct D {
>         void map();
>     }
>     D d;
> 
> to call
> 
>     (gdb) print d.map()
> 
> where previously this would have been a syntax error.
> 
> Tested on gdb.cp/*.exp
> 
> gdb/ChangeLog:
> 
>         * c-exp.y (lex_one_token, classify_name, yylex): Don't classify
>         names after a structop as a filename
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.cp/filename.cc, gdb.cp/filename.exp: Test that member
>         functions with the same name as an include file are parsed
>         correctly.
> ---
> 
> Fix completion token to only appear on completion.
> 
>  gdb/ChangeLog                     |  5 +++++
>  gdb/c-exp.y                       | 45 +++++++++++++++++++++------------------
>  gdb/testsuite/ChangeLog           |  6 ++++++
>  gdb/testsuite/gdb.cp/filename.cc  | 22 ++++++++++++++++++-
>  gdb/testsuite/gdb.cp/filename.exp | 20 +++++++++++++++--
>  5 files changed, 74 insertions(+), 24 deletions(-)

Thanks for the update, I have pushed it.

Simon