[RFA] Fix c++/14819 (implicit this)

[RFA] Fix c++/14819 (implicit this)

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 1218 bytes --]

Hi,

This is an attempt to address c++/14819. The simplest reproducer is:

class C {
public:
   int x;
   C():x(42) {}
   int f() {
     return C::x;
   }
} c;
int main() { c.f(); }

(gdb) print C::x
A syntax error in expression, near `x'.

This is happening because all qualified names like this go through 
value_aggregate_elt, which dispatches structs and classes to 
value_struct_elt_for_reference. That function does not allow references 
to non-static member data.

The solution I've implemented (as explained in the comments) is to treat 
a qualified name on non-static member with an implicit `this'. So the 
qualified name is looked up in the current this pointer (if any), and a 
value is computed as if the user had typed, "this->*(&QUALIFIED_NAME)".

No regressions on native x86_64-linux or native-gdbserver.

Comments, questions, concerns?
Keith

ChangeLog
2013-11-05  Keith Seitz  <keiths@redhat.com>

	PR c++/14819
	* valops.c (value_struct_elt_for_reference): If we get
	a non-static field, try to get a value based on the
	an implicit this pointer.

testsuite/ChangeLog
2013-11-05  Keith Seitz  <keiths@redhat.com>

	PR c++/14819
	* gdb.cp/impl-this.cc: New file.
	* gdb.cp/impl-this.exp: New file.


[-- Attachment #2: implicit-this.patch --]
[-- Type: text/x-patch, Size: 5234 bytes --]

diff --git a/gdb/valops.c b/gdb/valops.c
index 8bff686..2f3ead8 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3131,7 +3131,32 @@ value_struct_elt_for_reference (struct type *domain, int offset,
 	  else if (noside == EVAL_AVOID_SIDE_EFFECTS)
 	    return allocate_value (TYPE_FIELD_TYPE (t, i));
 	  else
-	    error (_("Cannot reference non-static field \"%s\""), name);
+	    {
+	      /* Try to evaluate NAME as a qualified name with implicit
+		 this pointer.  In this case, attempt to return the
+		 equivalent to `this->*(&TYPE::NAME)'.  */
+	      v = value_of_this_silent (current_language);
+	      if (v != NULL)
+		{
+		  struct value *ptr;
+		  long mem_offset;
+		  struct type *type, *tmp;
+
+		  ptr = value_aggregate_elt (t, name, NULL, 1, noside);
+		  type = check_typedef (value_type (ptr));
+		  gdb_assert (type != NULL
+			      && TYPE_CODE (type) == TYPE_CODE_MEMBERPTR);
+		  tmp = lookup_pointer_type (TYPE_DOMAIN_TYPE (type));
+		  v = value_cast_pointers (tmp, v, 1);
+		  mem_offset = value_as_long (ptr);
+		  tmp = lookup_pointer_type (TYPE_TARGET_TYPE (type));
+		  result = value_from_pointer (tmp,
+					       value_as_long (v) + mem_offset);
+		  return value_ind (result);
+		}
+
+	      error (_("Cannot reference non-static field \"%s\""), name);
+	    }
 	}
     }
 
diff --git a/gdb/testsuite/gdb.cp/impl-this.cc b/gdb/testsuite/gdb.cp/impl-this.cc
new file mode 100644
index 0000000..5b52f24
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/impl-this.cc
@@ -0,0 +1,52 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+class A
+{
+public:
+  int a;
+  int x, y, z;
+  A (void) : a (11), x (14), y (15), z (16) {}
+};
+
+class B : public A
+{
+public:
+  int a, b;
+  int x, y;
+  int solo;
+  B (void) : a (21), b (22), x (24), y (25), solo (100) {}
+};
+
+class C : public B
+{
+public:
+  int a, b, c;
+  int x;
+  C (void) : a (31), b (32), c (33), x (34) {}
+  int f (void) const
+  {
+    return (C::a + B::a + C::a - C::b - B::b + C::c
+	    - C::x - B::x - C::x + B::y + A::y - A::z);  /* break here */
+  }
+} c;
+
+int
+main (void)
+{
+  return c.f();
+}
diff --git a/gdb/testsuite/gdb.cp/impl-this.exp b/gdb/testsuite/gdb.cp/impl-this.exp
new file mode 100644
index 0000000..679b8ce
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/impl-this.exp
@@ -0,0 +1,67 @@
+# Copyright 2013 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/>.
+
+# This file is part of the gdb testsuite
+
+# Test expressions which assume an implicit "this" with a qualified
+# name.
+
+if {[skip_cplus_tests]} { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+if {![runto_main]} {
+    perror "couldn't run to main"
+    continue
+}
+
+gdb_test "print A::a" "Cannot reference non-static field \"a\""
+gdb_test "print A::b" "There is no field named b"
+
+gdb_breakpoint [gdb_get_line_number "break here"]
+gdb_continue_to_breakpoint "run to C::f"
+
+gdb_test "print A::a" "= 11"
+gdb_test "print A::b" "There is no field named b"
+gdb_test "print A::c" "There is no field named c"
+gdb_test "print A::x" "= 14"
+gdb_test "print A::y" "= 15"
+gdb_test "print A::z" "= 16"
+
+gdb_test "print B::a" "= 21"
+gdb_test "print B::b" "= 22"
+gdb_test "print B::c" "There is no field named c"
+gdb_test "print B::x" "= 24"
+gdb_test "print B::y" "= 25"
+gdb_test "print B::z" "= 16"; # B inherits from A!
+gdb_test "print B::solo" "= 100"
+
+gdb_test "print C::a" "= 31"
+gdb_test "print C::b" "= 32"
+gdb_test "print C::c" "= 33"
+gdb_test "print C::x" "= 34"
+gdb_test "print C::y" "= 25"; # C inherits from B!
+gdb_test "print C::z" "= 16"; # C inherits from B and B from A!
+
+gdb_test "print a" "= 31"
+gdb_test "print b" "= 32"
+gdb_test "print c" "= 33"
+gdb_test "print x" "= 34"
+gdb_test "print y" "= 25"; # C inherits from B!
+gdb_test "print z" "= 16"; # C inherits from B and B from A!

Re: [RFA] Fix c++/14819 (implicit this)

[Jan Kratochvil]

Hi Keith,

while the solution seems pretty tricky (some more comments what each line does
would be helpful) it does not work for 'p B::i' and 'p C::i'.  But these could
be fixed I guess, I did not try to do so.

------------------------------------------------------------------------------
class A {
public:
  int i;
  A(int i_):i(i_) {}
};
class B:public A { public: B():A(1) {} };
class C:public A { public: C():A(2) {} };
class D:public B,public C {
public:
  int f() {
    return B::i + C::i;
    // return i; // error: reference to ‘i’ is ambiguous
    // return A::i; // error: ‘A’ is an ambiguous base of ‘D’
  }
};
D d;
int main() { return d.f(); }
------------------------------------------------------------------------------
(gdb) p B::i
base class 'A' is ambiguous in type 'D'
 - this is bug
(gdb) p C::i
base class 'A' is ambiguous in type 'D'
 - this is bug
(gdb) p A::i
base class 'A' is ambiguous in type 'D'
 - this is correct
(gdb) p i
$1 = 2
 - this seems already filed as:
	ambiguous using reference is not error
	http://sourceware.org/bugzilla/show_bug.cgi?id=11540
------------------------------------------------------------------------------


On Tue, 05 Nov 2013 22:08:58 +0100, Keith Seitz wrote:
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -3131,7 +3131,32 @@ value_struct_elt_for_reference (struct type *domain, int offset,
>  	  else if (noside == EVAL_AVOID_SIDE_EFFECTS)
>  	    return allocate_value (TYPE_FIELD_TYPE (t, i));
>  	  else
> -	    error (_("Cannot reference non-static field \"%s\""), name);
> +	    {
> +	      /* Try to evaluate NAME as a qualified name with implicit
> +		 this pointer.  In this case, attempt to return the
> +		 equivalent to `this->*(&TYPE::NAME)'.  */
> +	      v = value_of_this_silent (current_language);
> +	      if (v != NULL)
> +		{
> +		  struct value *ptr;
> +		  long mem_offset;

LONGEST, it is returned by value_as_long.

> +		  struct type *type, *tmp;
> +
> +		  ptr = value_aggregate_elt (t, name, NULL, 1, noside);

I cannot reproduce it but I find it a bit fragile, value_aggregate_elt can
return NULL (although it probably would not get here in such case).
At least gdb_assert would be good.


> +		  type = check_typedef (value_type (ptr));
> +		  gdb_assert (type != NULL
> +			      && TYPE_CODE (type) == TYPE_CODE_MEMBERPTR);
> +		  tmp = lookup_pointer_type (TYPE_DOMAIN_TYPE (type));
> +		  v = value_cast_pointers (tmp, v, 1);
> +		  mem_offset = value_as_long (ptr);
> +		  tmp = lookup_pointer_type (TYPE_TARGET_TYPE (type));
> +		  result = value_from_pointer (tmp,
> +					       value_as_long (v) + mem_offset);
> +		  return value_ind (result);
> +		}
> +
> +	      error (_("Cannot reference non-static field \"%s\""), name);
> +	    }
>  	}
>      }


Thanks,
Jan

Re: [RFA] Fix c++/14819 (implicit this)

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> +	      /* Try to evaluate NAME as a qualified name with implicit
Keith> +		 this pointer.  In this case, attempt to return the
Keith> +		 equivalent to `this->*(&TYPE::NAME)'.  */

Nice approach, I totally didn't think of it.

I think it would be good to have a test case involving a virtual base
class, preferably one where the virtual base ends up in two different
places in different classes (and then test both cases).

Keith> +		  tmp = lookup_pointer_type (TYPE_TARGET_TYPE (type));
Keith> +		  result = value_from_pointer (tmp,
Keith> +					       value_as_long (v) + mem_offset);
Keith> +		  return value_ind (result);

Here I suspect it would be better to avoid the value_ind for some values
of "noside".  Otherwise I think we could end up reading memory
needlessly (by trying to compute the correct enclosing type).

Tom

Re: [RFA] Fix c++/14819 (implicit this)

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 2134 bytes --]

On 11/06/2013 09:42 AM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
> Keith> +		  tmp = lookup_pointer_type (TYPE_TARGET_TYPE (type));
> Keith> +		  result = value_from_pointer (tmp,
> Keith> +					       value_as_long (v) + mem_offset);
> Keith> +		  return value_ind (result);
>
> Here I suspect it would be better to avoid the value_ind for some values
> of "noside".  Otherwise I think we could end up reading memory
> needlessly (by trying to compute the correct enclosing type).

I'm not entirely sure it is possible to actually get EVAL_SKIP here 
(EVAL_AVOID_SIDE_EFFECTS is handled already), but I've changed it to 
"bail" if we see anything other than EVAL_NORMAL.

I've also discovered a few other bugs related to this and added tests 
for those. This is concerning finding base classes which are templates.

As I'm sure you knwo, the code currently finds baseclasses via 
lookup_symbol returning a symobl for the constructor. This is what the 
workarounds in classify_name (and now classify_inner_name) are all 
about. These are necessary evils, unfortunately.

Since template constructors look like "CLASS<template, parameter, 
list>::CLASS (args)", lookup_symbol does not find the constructor.

So I've added a new function, find_type_baseclass_by_name which searches 
the base classes of the given type for a base class of the given name. 
Calling this in classify_inner_name in the appropriate places "makes 
everything work." (TM)

How does this look?

Keith

ChangeLog
2013-11-13  Keith Seitz  <keiths@redhat.com>

	PR c++/14819
	* c-exp.y (classify_inner_name): If no matching symbol was
	found, try looking up the token as a base class.
	Likewise if a constructor was found.
	* cp-namespace.c (find_type_baseclass_by_name): New function.
	* cp-support.h (find_type_baseclass_by_name): Declare.
	* valops.c (value_struct_elt_for_reference): If we get
	a non-static field, try to get a value based on the
	an implicit this pointer.

testsuite/ChangeLog
2013-11-13  Keith Seitz  <keiths@redhat.com>

	PR c++/14819
	* gdb.cp/impl-this.cc: New file.
	* gdb.cp/impl-this.exp: New file.



[-- Attachment #2: implicit-this-2.patch --]
[-- Type: text/x-patch, Size: 10448 bytes --]

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 77713dd..3ab8e32 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -2948,13 +2948,39 @@ classify_inner_name (const struct block *block, struct type *context)
 
   copy = copy_name (yylval.ssym.stoken);
   yylval.ssym.sym = cp_lookup_nested_symbol (type, copy, block);
+
+  /* If no symbol was found, search for a matching base class named
+     COPY.  This will allow users to enter qualified names of class members
+     relative to the `this' pointer.  */
   if (yylval.ssym.sym == NULL)
-    return ERROR;
+    {
+      struct type *base_type = find_type_baseclass_by_name (type, copy);
+
+      if (base_type != NULL)
+	{
+	  yylval.tsym.type = base_type;
+	  return TYPENAME;
+	}
+
+      return ERROR;
+    }
 
   switch (SYMBOL_CLASS (yylval.ssym.sym))
     {
     case LOC_BLOCK:
     case LOC_LABEL:
+      /* cp_lookup_nested_symbol might have accidentally found a constructor
+	 named COPY when we really wanted a base class of the same name.
+	 Double-check this case by looking for a base class.  */
+      {
+	struct type *base_type = find_type_baseclass_by_name (type, copy);
+
+	if (base_type != NULL)
+	  {
+	    yylval.tsym.type = base_type;
+	    return TYPENAME;
+	  }
+      }
       return ERROR;
 
     case LOC_TYPEDEF:
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 36134c0..adecf21 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -703,6 +703,33 @@ lookup_symbol_file (const char *name,
   return sym;
 }
 
+/* Search through the base classes of PARENT_TYPE for a base class
+   named NAME and return its type.  If not found, return NULL.  */
+
+struct type *
+find_type_baseclass_by_name (struct type *parent_type, const char *name)
+{
+  int i;
+
+  for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i)
+    {
+      struct type *type = TYPE_BASECLASS (parent_type, i);
+      const char *base_name = TYPE_BASECLASS_NAME (parent_type, i);
+
+      if (base_name == NULL)
+	continue;
+
+      if (streq (base_name, name))
+	return type;
+
+      type = find_type_baseclass_by_name (type, name);
+      if (type != NULL)
+	return type;
+    }
+
+  return NULL;
+}
+
 /* Search through the base classes of PARENT_TYPE for a symbol named
    NAME in block BLOCK.  */
 
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 0f2cebb..4358b23 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -220,6 +220,11 @@ extern struct symbol *cp_lookup_nested_symbol (struct type *parent_type,
 
 struct type *cp_lookup_transparent_type (const char *name);
 
+/* See description in cp-namespace.c.  */
+
+struct type *find_type_baseclass_by_name (struct type *parent_type,
+					  const char *name);
+
 /* Functions from cp-name-parser.y.  */
 
 extern struct demangle_parse_info *cp_demangled_name_to_comp
diff --git a/gdb/valops.c b/gdb/valops.c
index 8bff686..780d6be 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3128,10 +3128,35 @@ value_struct_elt_for_reference (struct type *domain, int offset,
 	    return value_from_longest
 	      (lookup_memberptr_type (TYPE_FIELD_TYPE (t, i), domain),
 	       offset + (LONGEST) (TYPE_FIELD_BITPOS (t, i) >> 3));
-	  else if (noside == EVAL_AVOID_SIDE_EFFECTS)
+	  else if (noside != EVAL_NORMAL)
 	    return allocate_value (TYPE_FIELD_TYPE (t, i));
 	  else
-	    error (_("Cannot reference non-static field \"%s\""), name);
+	    {
+	      /* Try to evaluate NAME as a qualified name with implicit
+		 this pointer.  In this case, attempt to return the
+		 equivalent to `this->*(&TYPE::NAME)'.  */
+	      v = value_of_this_silent (current_language);
+	      if (v != NULL)
+		{
+		  struct value *ptr;
+		  long mem_offset;
+		  struct type *type, *tmp;
+
+		  ptr = value_aggregate_elt (domain, name, NULL, 1, noside);
+		  type = check_typedef (value_type (ptr));
+		  gdb_assert (type != NULL
+			      && TYPE_CODE (type) == TYPE_CODE_MEMBERPTR);
+		  tmp = lookup_pointer_type (TYPE_DOMAIN_TYPE (type));
+		  v = value_cast_pointers (tmp, v, 1);
+		  mem_offset = value_as_long (ptr);
+		  tmp = lookup_pointer_type (TYPE_TARGET_TYPE (type));
+		  result = value_from_pointer (tmp,
+					       value_as_long (v) + mem_offset);
+		  return value_ind (result);
+		}
+
+	      error (_("Cannot reference non-static field \"%s\""), name);
+	    }
 	}
     }
 
diff --git a/gdb/testsuite/gdb.cp/impl-this.cc b/gdb/testsuite/gdb.cp/impl-this.cc
new file mode 100644
index 0000000..abe6ffd
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/impl-this.cc
@@ -0,0 +1,99 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+#ifdef DEBUG
+#include <stdio.h>
+#endif
+
+template <typename T>
+class A
+{
+public:
+  T i;
+  T z;
+  A () : i (1), z (10) {}
+};
+
+template <typename T>
+class B : public virtual A<T>
+{
+public:
+  T i;
+  B () : i (2) {}
+};
+
+class C : public virtual A<int>
+{
+public:
+  int i;
+  int c;
+  C () : i (3), c (30) {}
+};
+
+class D : public B<int>, public C
+{
+public:
+  int i;
+  int x;
+  D () : i (4), x (40) {}
+
+#ifdef DEBUG
+#define SUM(X)					\
+  do						\
+    {						\
+      sum += (X);				\
+      printf ("" #X " = %d\n", (X));		\
+    }						\
+  while (0)
+#else
+#define SUM(X) sum += (X)
+#endif
+
+int
+f (void)
+  {
+    int sum = 0;
+
+    SUM (i);
+    SUM (D::i);
+    SUM (D::B<int>::i);
+    SUM (B<int>::i);
+    SUM (D::C::i);
+    SUM (C::i);
+    SUM (D::B<int>::A<int>::i);
+    SUM (B<int>::A<int>::i);
+    SUM (A<int>::i);
+    SUM (D::C::A<int>::i);
+    SUM (C::A<int>::i);
+    SUM (D::x);
+    SUM (x);
+    SUM (D::C::c);
+    SUM (C::c);
+    SUM (c);
+    SUM (D::A<int>::i);
+
+    return sum;
+  }
+};
+
+int
+main (void)
+{
+  D d;
+
+  return d.f ();
+}
diff --git a/gdb/testsuite/gdb.cp/impl-this.exp b/gdb/testsuite/gdb.cp/impl-this.exp
new file mode 100644
index 0000000..7005d4c
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/impl-this.exp
@@ -0,0 +1,91 @@
+# Copyright 2013 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/>.
+
+# This file is part of the gdb testsuite
+
+# Test expressions which assume an implicit "this" with a qualified
+# name.
+
+if {[skip_cplus_tests]} { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+# First test expressions when there is no context.
+gdb_test "print i" "No symbol \"i\" in current context."
+gdb_test "print D::i" "Cannot reference non-static field \"i\""
+gdb_test "print D::B<int>::i" "Cannot reference non-static field \"i\""
+gdb_test "print B<int>::i" "Cannot reference non-static field \"i\""
+gdb_test "print D::C::i" "Cannot reference non-static field \"i\""
+gdb_test "print C::i" "Cannot reference non-static field \"i\""
+gdb_test "print D::B<int>::A<int>::i" "Cannot reference non-static field \"i\""
+gdb_test "print B<int>::A<int>::i" "Cannot reference non-static field \"i\""
+gdb_test "print A<int>::i" "Cannot reference non-static field \"i\""
+gdb_test "print D::C::A<int>::i" "Cannot reference non-static field \"i\""
+gdb_test "print C::A<int>::i" "Cannot reference non-static field \"i\""
+gdb_test "print D::x" "Cannot reference non-static field \"x\""
+gdb_test "print x" "No symbol \"x\" in current context."
+gdb_test "print D::C::c" "Cannot reference non-static field \"c\""
+gdb_test "print C::c" "Cannot reference non-static field \"c\""
+gdb_test "print c" "No symbol \"c\" in current context."
+gdb_test "print D::A<int>::i" "Cannot reference non-static field \"i\""
+
+# Run to D::f.
+if {![runto_main]} {
+    perror "couldn't run to main"
+    continue
+}
+
+gdb_breakpoint "D::f"
+gdb_continue_to_breakpoint "continue to D::f"
+
+# Now test valid expressions in the class hierarchy for D.
+gdb_test "print i" "= 4"
+gdb_test "print D::i" "= 4"
+gdb_test "print D::B<int>::i" "= 2"
+gdb_test "print B<int>::i" "= 2"
+gdb_test "print D::C::i" "= 3"
+gdb_test "print C::i" "= 3"
+gdb_test "print D::B<int>::A<int>::i" "= 1"
+gdb_test "print B<int>::A<int>::i" "= 1"
+gdb_test "print A<int>::i" "= 1"
+gdb_test "print D::C::A<int>::i" "= 1"
+gdb_test "print C::A<int>::i" "= 1"
+gdb_test "print D::x" "= 40"
+gdb_test "print x" "= 40"
+gdb_test "print D::C::c" "= 30"
+gdb_test "print C::c" "= 30"
+gdb_test "print c" "= 30"
+gdb_test "print D::A<int>::i" "= 1"
+
+# Test some invalid expressions
+gdb_test "print D::B<int>::c" "There is no field named c"
+gdb_test "print D::B<int>::A<int>::c" "There is no field named c"
+gdb_test "print D::C::A<int>::c" "There is no field named c"
+gdb_test "print B<int>::c" "There is no field named c"
+gdb_test "print B<int>::A<int>::c" "There is no field named c"
+gdb_test "print C::A<int>::c" "There is no field named c"
+gdb_test "print D::B<int>::x" "There is no field named x"
+gdb_test "print D::B<int>::A<int>::x" "There is no field named x"
+gdb_test "print B<int>::x" "There is no field named x"
+gdb_test "print B<int>::A<int>::x" "There is no field named x"
+gdb_test "print D::C::x" "There is no field named x"
+gdb_test "print C::x" "There is no field named x"
+gdb_test "print D::C::A<int>::x" "There is no field named x"
+gdb_test "print C::A<int>::x" "There is no field named x"
+

Re: [RFA] Fix c++/14819 (implicit this)

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> I'm not entirely sure it is possible to actually get EVAL_SKIP
Keith> here (EVAL_AVOID_SIDE_EFFECTS is handled already), but I've
Keith> changed it to "bail" if we see anything other than EVAL_NORMAL.

Yeah.  Ideally I think we wouldn't let EVAL_* escape eval.c.  Not your
problem though.

Keith> So I've added a new function, find_type_baseclass_by_name which
Keith> searches the base classes of the given type for a base class of the
Keith> given name. Calling this in classify_inner_name in the appropriate
Keith> places "makes everything work." (TM)

This seems like a fine idea to me.

Keith> +      /* cp_lookup_nested_symbol might have accidentally found a constructor
Keith> +	 named COPY when we really wanted a base class of the same name.
Keith> +	 Double-check this case by looking for a base class.  */
Keith> +      {
Keith> +	struct type *base_type = find_type_baseclass_by_name (type, copy);
Keith> +
Keith> +	if (base_type != NULL)
Keith> +	  {
Keith> +	    yylval.tsym.type = base_type;
Keith> +	    return TYPENAME;
Keith> +	  }

I wonder what happens here if you actually do want the constructor.
Like "print B::B".  Will it still find the baseclass?

Keith> +/* Search through the base classes of PARENT_TYPE for a base class
Keith> +   named NAME and return its type.  If not found, return NULL.  */
Keith> +
Keith> +struct type *
Keith> +find_type_baseclass_by_name (struct type *parent_type, const char *name)
Keith> +{
Keith> +  int i;
Keith> +
Keith> +  for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i)

Probably this function requires check_typedef sprinkled liberally
around.

Keith> +    {
Keith> +      struct type *type = TYPE_BASECLASS (parent_type, i);
Keith> +      const char *base_name = TYPE_BASECLASS_NAME (parent_type, i);
Keith> +
Keith> +      if (base_name == NULL)
Keith> +	continue;
Keith> +
Keith> +      if (streq (base_name, name))
Keith> +	return type;

It is possible to have ambiguous base classes?
And if so, where is that diagnosed?

Another horrible thought is what does "this->typedef_name::field" mean?
Anything?  What is typedef_name is a typedef for one of the base classes?

Keith> +gdb_test "print D::i" "Cannot reference non-static field \"i\""

Keith> +gdb_test "print D::i" "= 4"

Duplicated test names.
I think there are more than one.
It's fine to just wrap the sequences in with_test_prefix though.

Tom

Re: [RFA] Fix c++/14819 (implicit this)

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 4091 bytes --]

On 11/15/2013 09:48 AM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
> Keith> +      /* cp_lookup_nested_symbol might have accidentally found a constructor
> Keith> +	 named COPY when we really wanted a base class of the same name.
> Keith> +	 Double-check this case by looking for a base class.  */
> Keith> +      {
> Keith> +	struct type *base_type = find_type_baseclass_by_name (type, copy);
> Keith> +
> Keith> +	if (base_type != NULL)
> Keith> +	  {
> Keith> +	    yylval.tsym.type = base_type;
> Keith> +	    return TYPENAME;
> Keith> +	  }
>
> I wonder what happens here if you actually do want the constructor.
> Like "print B::B".  Will it still find the baseclass?

cp_lookup_nested_symbol will find the constructor's symbol, and this 
whole block is bypassed.

> Keith> +/* Search through the base classes of PARENT_TYPE for a base class
> Keith> +   named NAME and return its type.  If not found, return NULL.  */
> Keith> +
> Keith> +struct type *
> Keith> +find_type_baseclass_by_name (struct type *parent_type, const char *name)
> Keith> +{
> Keith> +  int i;
> Keith> +
> Keith> +  for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i)
>
> Probably this function requires check_typedef sprinkled liberally
> around.
>
> Keith> +    {
> Keith> +      struct type *type = TYPE_BASECLASS (parent_type, i);
> Keith> +      const char *base_name = TYPE_BASECLASS_NAME (parent_type, i);
> Keith> +
> Keith> +      if (base_name == NULL)
> Keith> +	continue;
> Keith> +
> Keith> +      if (streq (base_name, name))
> Keith> +	return type;
>
> It is possible to have ambiguous base classes?
> And if so, where is that diagnosed?

Yes, it is possible, but we're not interested in ascertaining that here. 
As far as the parser is concerned, we simply need to know whether the 
string represents a type name.

The case of ambiguous base classes is handled by value_cast_pointers 
which calls search_struct_field, which notices the ambiguity and errors.

This probably isn't ideal should some other developer use this function 
later for something other than parsing, but it is currently sufficient.

I've added tests for ambiguous bases.

> Another horrible thought is what does "this->typedef_name::field" mean?
> Anything?  What is typedef_name is a typedef for one of the base classes?

If I'm reading the standard correctly, this is not possible (at least 
not in the way I think you mean). 3.4.5 Class Member Access #4 says:

"If the id-expression in a class member access is a qualified-id of the 
form:

   class-name-or-namespace-name::...

the class-name-or-namespace-name following the . or -> operator is first 
looked up in the class of the object expression and the name, if found, 
is used. Otherwise it is looked up in the context of the entire 
postfix-expression." -- n3290 draft

So for example (according to my reading),

class A {};
typedef A AA;
class B : public AA {};

B::A::whatever <- valid
B::AA::whatever <- not valid - "B" does not define anything called "AA".

Now if the user adds a typedef into B for AA, then the above expression 
would be valid.

I've added tests to make sure that gdb enforces this behavior (which it 
already does).

> Keith> +gdb_test "print D::i" "Cannot reference non-static field \"i\""
>
> Keith> +gdb_test "print D::i" "= 4"
>
> Duplicated test names.
> I think there are more than one.
> It's fine to just wrap the sequences in with_test_prefix though.

Fixed.

Keith

ChangeLog
2013-11-21  Keith Seitz  <keiths@redhat.com>

	PR c++/14819
	* c-exp.y (classify_inner_name): If no matching symbol was
	found, try looking up the token as a base class.
	Likewise if a constructor was found.
	* cp-namespace.c (find_type_baseclass_by_name): New function.
	* cp-support.h (find_type_baseclass_by_name): Declare.
	* valops.c (value_struct_elt_for_reference): If we get
	a non-static field, try to get a value based on the
	current instance, if any.

testsuite/ChangeLog
2013-11-21  Keith Seitz  <keiths@redhat.com>

	PR c++/14819
	* gdb.cp/impl-this.cc: New file.
	* gdb.cp/impl-this.exp: New file.



[-- Attachment #2: implicit-this-2.patch --]
[-- Type: text/x-patch, Size: 12828 bytes --]

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 5d4cd81..03af9e7 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -2960,13 +2960,39 @@ classify_inner_name (const struct block *block, struct type *context)
 
   copy = copy_name (yylval.ssym.stoken);
   yylval.ssym.sym = cp_lookup_nested_symbol (type, copy, block);
+
+  /* If no symbol was found, search for a matching base class named
+     COPY.  This will allow users to enter qualified names of class members
+     relative to the `this' pointer.  */
   if (yylval.ssym.sym == NULL)
-    return ERROR;
+    {
+      struct type *base_type = find_type_baseclass_by_name (type, copy);
+
+      if (base_type != NULL)
+	{
+	  yylval.tsym.type = base_type;
+	  return TYPENAME;
+	}
+
+      return ERROR;
+    }
 
   switch (SYMBOL_CLASS (yylval.ssym.sym))
     {
     case LOC_BLOCK:
     case LOC_LABEL:
+      /* cp_lookup_nested_symbol might have accidentally found a constructor
+	 named COPY when we really wanted a base class of the same name.
+	 Double-check this case by looking for a base class.  */
+      {
+	struct type *base_type = find_type_baseclass_by_name (type, copy);
+
+	if (base_type != NULL)
+	  {
+	    yylval.tsym.type = base_type;
+	    return TYPENAME;
+	  }
+      }
       return ERROR;
 
     case LOC_TYPEDEF:
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 36134c0..a23dec1 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -703,6 +703,34 @@ lookup_symbol_file (const char *name,
   return sym;
 }
 
+/* Search through the base classes of PARENT_TYPE for a base class
+   named NAME and return its type.  If not found, return NULL.  */
+
+struct type *
+find_type_baseclass_by_name (struct type *parent_type, const char *name)
+{
+  int i;
+
+  CHECK_TYPEDEF (parent_type);
+  for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i)
+    {
+      struct type *type = TYPE_BASECLASS (parent_type, i);
+      const char *base_name = TYPE_BASECLASS_NAME (parent_type, i);
+
+      if (base_name == NULL)
+	continue;
+
+      if (streq (base_name, name))
+	return type;
+
+      type = find_type_baseclass_by_name (type, name);
+      if (type != NULL)
+	return type;
+    }
+
+  return NULL;
+}
+
 /* Search through the base classes of PARENT_TYPE for a symbol named
    NAME in block BLOCK.  */
 
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 0f2cebb..4358b23 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -220,6 +220,11 @@ extern struct symbol *cp_lookup_nested_symbol (struct type *parent_type,
 
 struct type *cp_lookup_transparent_type (const char *name);
 
+/* See description in cp-namespace.c.  */
+
+struct type *find_type_baseclass_by_name (struct type *parent_type,
+					  const char *name);
+
 /* Functions from cp-name-parser.y.  */
 
 extern struct demangle_parse_info *cp_demangled_name_to_comp
diff --git a/gdb/valops.c b/gdb/valops.c
index 4fc57ec..8e7b16f 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3128,10 +3128,35 @@ value_struct_elt_for_reference (struct type *domain, int offset,
 	    return value_from_longest
 	      (lookup_memberptr_type (TYPE_FIELD_TYPE (t, i), domain),
 	       offset + (LONGEST) (TYPE_FIELD_BITPOS (t, i) >> 3));
-	  else if (noside == EVAL_AVOID_SIDE_EFFECTS)
+	  else if (noside != EVAL_NORMAL)
 	    return allocate_value (TYPE_FIELD_TYPE (t, i));
 	  else
-	    error (_("Cannot reference non-static field \"%s\""), name);
+	    {
+	      /* Try to evaluate NAME as a qualified name with implicit
+		 this pointer.  In this case, attempt to return the
+		 equivalent to `this->*(&TYPE::NAME)'.  */
+	      v = value_of_this_silent (current_language);
+	      if (v != NULL)
+		{
+		  struct value *ptr;
+		  long mem_offset;
+		  struct type *type, *tmp;
+
+		  ptr = value_aggregate_elt (domain, name, NULL, 1, noside);
+		  type = check_typedef (value_type (ptr));
+		  gdb_assert (type != NULL
+			      && TYPE_CODE (type) == TYPE_CODE_MEMBERPTR);
+		  tmp = lookup_pointer_type (TYPE_DOMAIN_TYPE (type));
+		  v = value_cast_pointers (tmp, v, 1);
+		  mem_offset = value_as_long (ptr);
+		  tmp = lookup_pointer_type (TYPE_TARGET_TYPE (type));
+		  result = value_from_pointer (tmp,
+					       value_as_long (v) + mem_offset);
+		  return value_ind (result);
+		}
+
+	      error (_("Cannot reference non-static field \"%s\""), name);
+	    }
 	}
     }
 
diff --git a/gdb/testsuite/gdb.cp/impl-this.cc b/gdb/testsuite/gdb.cp/impl-this.cc
new file mode 100644
index 0000000..6be0ddf
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/impl-this.cc
@@ -0,0 +1,135 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+#ifdef DEBUG
+#include <stdio.h>
+#endif
+
+template <typename T>
+class A
+{
+public:
+  T i;
+  T z;
+  A () : i (1), z (10) {}
+};
+
+template <typename T>
+class B : public virtual A<T>
+{
+public:
+  T i;
+  T common;
+  B () : i (2), common (200) {}
+};
+
+typedef B<int> Bint;
+
+class C : public virtual A<int>
+{
+public:
+  int i;
+  int c;
+  int common;
+  C () : i (3), c (30), common (300) {}
+};
+
+class BB : public A<int>
+{
+public:
+  int i;
+  BB () : i (20) {}
+};
+
+class CC : public A<int>
+{
+public:
+  int i;
+  CC () : i (30) {}
+};
+
+class Ambig : public BB, public CC
+{
+public:
+  int i;
+  Ambig () : i (1000) {}
+};
+
+class D : public Bint, public C
+{
+public:
+  int i;
+  int x;
+  Ambig am;
+  D () : i (4), x (40) {}
+
+#ifdef DEBUG
+#define SUM(X)					\
+  do						\
+    {						\
+      sum += (X);				\
+      printf ("" #X " = %d\n", (X));		\
+    }						\
+  while (0)
+#else
+#define SUM(X) sum += (X)
+#endif
+
+int
+f (void)
+  {
+    int sum = 0;
+
+    SUM (i);
+    SUM (D::i);
+    SUM (D::B<int>::i);
+    SUM (B<int>::i);
+    SUM (D::C::i);
+    SUM (C::i);
+    SUM (D::B<int>::A<int>::i);
+    SUM (B<int>::A<int>::i);
+    SUM (A<int>::i);
+    SUM (D::C::A<int>::i);
+    SUM (C::A<int>::i);
+    SUM (D::x);
+    SUM (x);
+    SUM (D::C::c);
+    SUM (C::c);
+    SUM (c);
+    SUM (D::A<int>::i);
+    SUM (Bint::i);
+    //SUM (D::Bint::i);
+    //SUM (D::Bint::A<int>::i);
+    SUM (Bint::A<int>::i);
+    // ambiguous: SUM (common);
+    SUM (B<int>::common);
+    SUM (C::common);
+    SUM (am.i);
+    // ambiguous: SUM (am.A<int>::i);
+
+    return sum;
+  }
+};
+
+int
+main (void)
+{
+  Bint b;
+  D d;
+
+  return d.f () + b.i;
+}
diff --git a/gdb/testsuite/gdb.cp/impl-this.exp b/gdb/testsuite/gdb.cp/impl-this.exp
new file mode 100644
index 0000000..ce7c780
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/impl-this.exp
@@ -0,0 +1,130 @@
+# Copyright 2013 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/>.
+
+# This file is part of the gdb testsuite
+
+# Test expressions which assume an implicit "this" with a qualified
+# name.
+
+if {[skip_cplus_tests]} { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+# First test expressions when there is no context.
+with_test_prefix "before run" {
+    gdb_test "print i" "No symbol \"i\" in current context."
+    gdb_test "print D::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::B<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print B<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::C::i" "Cannot reference non-static field \"i\""
+    gdb_test "print C::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::B<int>::A<int>::i" \
+	"Cannot reference non-static field \"i\""
+    gdb_test "print B<int>::A<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print A<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::C::A<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print C::A<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::x" "Cannot reference non-static field \"x\""
+    gdb_test "print x" "No symbol \"x\" in current context."
+    gdb_test "print D::C::c" "Cannot reference non-static field \"c\""
+    gdb_test "print C::c" "Cannot reference non-static field \"c\""
+    gdb_test "print c" "No symbol \"c\" in current context."
+    gdb_test "print D::A<int>::i" "Cannot reference non-static field \"i\""
+}
+
+# Run to D::f.
+if {![runto_main]} {
+    perror "couldn't run to main"
+    continue
+}
+
+gdb_breakpoint "D::f"
+gdb_continue_to_breakpoint "continue to D::f"
+
+# Now test valid expressions in the class hierarchy for D.
+with_test_prefix "at D::f (valid expressions)" {
+    gdb_test "print i" "= 4"
+    gdb_test "print D::i" "= 4"
+    gdb_test "print D::B<int>::i" "= 2"
+    gdb_test "print B<int>::i" "= 2"
+    gdb_test "print D::Bint::i" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print Bint::i" "= 2"
+    gdb_test "print D::C::i" "= 3"
+    gdb_test "print C::i" "= 3"
+    gdb_test "print D::B<int>::A<int>::i" "= 1"
+    gdb_test "print B<int>::A<int>::i" "= 1"
+    gdb_test "print D::Bint::A<int>::i" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print Bint::A<int>::i" "= 1"
+    gdb_test "print A<int>::i" "= 1"
+    gdb_test "print D::C::A<int>::i" "= 1"
+    gdb_test "print C::A<int>::i" "= 1"
+    gdb_test "print D::x" "= 40"
+    gdb_test "print x" "= 40"
+    gdb_test "print D::C::c" "= 30"
+    gdb_test "print C::c" "= 30"
+    gdb_test "print c" "= 30"
+    gdb_test "print D::A<int>::i" "= 1"
+}
+
+# Test some invalid expressions
+with_test_prefix "at D::f (invalid expressions)" {
+    gdb_test "print D::B<int>::c" "There is no field named c"
+    gdb_test "print D::B<int>::A<int>::c" "There is no field named c"
+    gdb_test "print D::Bint::c" \
+	"No type \"Bint\" within class or namespace \"D\"."
+
+    gdb_test "print D::Bint::A<int>::c" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print D::C::A<int>::c" "There is no field named c"
+    gdb_test "print B<int>::c" "There is no field named c"
+    gdb_test "print B<int>::A<int>::c" "There is no field named c"
+    gdb_test "print Bint::c" "There is no field named c"
+    gdb_test "print Bint::A<int>::c" "There is no field named c"
+    gdb_test "print C::A<int>::c" "There is no field named c"
+    gdb_test "print D::B<int>::x" "There is no field named x"
+    gdb_test "print D::B<int>::A<int>::x" "There is no field named x"
+    gdb_test "print D::Bint::x" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print D::Bint::A<int>::x" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print B<int>::x" "There is no field named x"
+    gdb_test "print B<int>::A<int>::x" "There is no field named x"
+    gdb_test "print Bint::x" "There is no field named x"
+    gdb_test "print Bint::A<int>::x" "There is no field named x"
+    gdb_test "print D::C::x" "There is no field named x"
+    gdb_test "print C::x" "There is no field named x"
+    gdb_test "print D::C::A<int>::x" "There is no field named x"
+    gdb_test "print C::A<int>::x" "There is no field named x"
+}
+
+# Test some ambiguous names
+with_test_prefix "at D::f (ambiguous names)" {
+    gdb_test "print B<int>::common" " = 200"
+    gdb_test "print Bint::common" " = 200"
+    gdb_test "print C::common" " = 300"
+    gdb_test "print am.i" " = 1000"
+    gdb_test "print am.A<int>::i" \
+	"base class 'A<int>' is ambiguous in type 'Ambig'"
+    gdb_test "print am.BB::A<int>::i" \
+	"base class 'A<int>' is ambiguous in type 'Ambig'"
+    gdb_test "print am.CC::A<int>::i" \
+	"base class 'A<int>' is ambiguous in type 'Ambig'"
+}

Re: [RFA] Fix c++/14819 (implicit this)

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> 2013-11-21  Keith Seitz  <keiths@redhat.com>
Keith> 	PR c++/14819
Keith> 	* c-exp.y (classify_inner_name): If no matching symbol was
Keith> 	found, try looking up the token as a base class.
Keith> 	Likewise if a constructor was found.
Keith> 	* cp-namespace.c (find_type_baseclass_by_name): New function.
Keith> 	* cp-support.h (find_type_baseclass_by_name): Declare.
Keith> 	* valops.c (value_struct_elt_for_reference): If we get
Keith> 	a non-static field, try to get a value based on the
Keith> 	current instance, if any.

Keith> testsuite/ChangeLog
Keith> 2013-11-21  Keith Seitz  <keiths@redhat.com>

Keith> 	PR c++/14819
Keith> 	* gdb.cp/impl-this.cc: New file.
Keith> 	* gdb.cp/impl-this.exp: New file.

Thanks, Keith.  I appreciate your research into my questions.

Keith> +struct type *
Keith> +find_type_baseclass_by_name (struct type *parent_type, const char *name)
Keith> +{
Keith> +  int i;
Keith> +
Keith> +  CHECK_TYPEDEF (parent_type);
Keith> +  for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i)
Keith> +    {
Keith> +      struct type *type = TYPE_BASECLASS (parent_type, i);

I think a check_typedef call is needed here.

Ok with that change.

Tom

Re: [RFA] Fix c++/14819 (implicit this)

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]

On 11/21/2013 01:06 PM, Tom Tromey wrote:
> Keith> +struct type *
> Keith> +find_type_baseclass_by_name (struct type *parent_type, const char *name)
> Keith> +{
> Keith> +  int i;
> Keith> +
> Keith> +  CHECK_TYPEDEF (parent_type);
> Keith> +  for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i)
> Keith> +    {
> Keith> +      struct type *type = TYPE_BASECLASS (parent_type, i);
>
> I think a check_typedef call is needed here.
>
> Ok with that change.

I've added that (it shouldn't hurt) and committed the patch (which I am 
also attaching for reference). Thank you for the review.

Keith

ChangeLog
2013-11-25  Keith Seitz  <keiths@redhat.com>

	PR c++/14819
	* c-exp.y (classify_inner_name): If no matching symbol was
	found, try looking up the token as a base class.
	Likewise if a constructor was found.
	* cp-namespace.c (find_type_baseclass_by_name): New function.
	* cp-support.h (find_type_baseclass_by_name): Declare.
	* valops.c (value_struct_elt_for_reference): If we get
	a non-static field, try to get a value based on the
	current instance, if any.

testsuite/ChangeLog
2013-11-25  Keith Seitz  <keiths@redhat.com>

	PR c++/14819
	* gdb.cp/impl-this.cc: New file.
	* gdb.cp/impl-this.exp: New file.



[-- Attachment #2: implicit-this-final.patch --]
[-- Type: text/x-patch, Size: 12844 bytes --]

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 5d4cd81..03af9e7 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -2960,13 +2960,39 @@ classify_inner_name (const struct block *block, struct type *context)
 
   copy = copy_name (yylval.ssym.stoken);
   yylval.ssym.sym = cp_lookup_nested_symbol (type, copy, block);
+
+  /* If no symbol was found, search for a matching base class named
+     COPY.  This will allow users to enter qualified names of class members
+     relative to the `this' pointer.  */
   if (yylval.ssym.sym == NULL)
-    return ERROR;
+    {
+      struct type *base_type = find_type_baseclass_by_name (type, copy);
+
+      if (base_type != NULL)
+	{
+	  yylval.tsym.type = base_type;
+	  return TYPENAME;
+	}
+
+      return ERROR;
+    }
 
   switch (SYMBOL_CLASS (yylval.ssym.sym))
     {
     case LOC_BLOCK:
     case LOC_LABEL:
+      /* cp_lookup_nested_symbol might have accidentally found a constructor
+	 named COPY when we really wanted a base class of the same name.
+	 Double-check this case by looking for a base class.  */
+      {
+	struct type *base_type = find_type_baseclass_by_name (type, copy);
+
+	if (base_type != NULL)
+	  {
+	    yylval.tsym.type = base_type;
+	    return TYPENAME;
+	  }
+      }
       return ERROR;
 
     case LOC_TYPEDEF:
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 36134c0..d0520bd 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -703,6 +703,34 @@ lookup_symbol_file (const char *name,
   return sym;
 }
 
+/* Search through the base classes of PARENT_TYPE for a base class
+   named NAME and return its type.  If not found, return NULL.  */
+
+struct type *
+find_type_baseclass_by_name (struct type *parent_type, const char *name)
+{
+  int i;
+
+  CHECK_TYPEDEF (parent_type);
+  for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i)
+    {
+      struct type *type = check_typedef (TYPE_BASECLASS (parent_type, i));
+      const char *base_name = TYPE_BASECLASS_NAME (parent_type, i);
+
+      if (base_name == NULL)
+	continue;
+
+      if (streq (base_name, name))
+	return type;
+
+      type = find_type_baseclass_by_name (type, name);
+      if (type != NULL)
+	return type;
+    }
+
+  return NULL;
+}
+
 /* Search through the base classes of PARENT_TYPE for a symbol named
    NAME in block BLOCK.  */
 
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 0f2cebb..4358b23 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -220,6 +220,11 @@ extern struct symbol *cp_lookup_nested_symbol (struct type *parent_type,
 
 struct type *cp_lookup_transparent_type (const char *name);
 
+/* See description in cp-namespace.c.  */
+
+struct type *find_type_baseclass_by_name (struct type *parent_type,
+					  const char *name);
+
 /* Functions from cp-name-parser.y.  */
 
 extern struct demangle_parse_info *cp_demangled_name_to_comp
diff --git a/gdb/valops.c b/gdb/valops.c
index 4fc57ec..8e7b16f 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3128,10 +3128,35 @@ value_struct_elt_for_reference (struct type *domain, int offset,
 	    return value_from_longest
 	      (lookup_memberptr_type (TYPE_FIELD_TYPE (t, i), domain),
 	       offset + (LONGEST) (TYPE_FIELD_BITPOS (t, i) >> 3));
-	  else if (noside == EVAL_AVOID_SIDE_EFFECTS)
+	  else if (noside != EVAL_NORMAL)
 	    return allocate_value (TYPE_FIELD_TYPE (t, i));
 	  else
-	    error (_("Cannot reference non-static field \"%s\""), name);
+	    {
+	      /* Try to evaluate NAME as a qualified name with implicit
+		 this pointer.  In this case, attempt to return the
+		 equivalent to `this->*(&TYPE::NAME)'.  */
+	      v = value_of_this_silent (current_language);
+	      if (v != NULL)
+		{
+		  struct value *ptr;
+		  long mem_offset;
+		  struct type *type, *tmp;
+
+		  ptr = value_aggregate_elt (domain, name, NULL, 1, noside);
+		  type = check_typedef (value_type (ptr));
+		  gdb_assert (type != NULL
+			      && TYPE_CODE (type) == TYPE_CODE_MEMBERPTR);
+		  tmp = lookup_pointer_type (TYPE_DOMAIN_TYPE (type));
+		  v = value_cast_pointers (tmp, v, 1);
+		  mem_offset = value_as_long (ptr);
+		  tmp = lookup_pointer_type (TYPE_TARGET_TYPE (type));
+		  result = value_from_pointer (tmp,
+					       value_as_long (v) + mem_offset);
+		  return value_ind (result);
+		}
+
+	      error (_("Cannot reference non-static field \"%s\""), name);
+	    }
 	}
     }
diff --git a/gdb/testsuite/gdb.cp/impl-this.cc b/gdb/testsuite/gdb.cp/impl-this.cc
new file mode 100644
index 0000000..6be0ddf
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/impl-this.cc
@@ -0,0 +1,135 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+#ifdef DEBUG
+#include <stdio.h>
+#endif
+
+template <typename T>
+class A
+{
+public:
+  T i;
+  T z;
+  A () : i (1), z (10) {}
+};
+
+template <typename T>
+class B : public virtual A<T>
+{
+public:
+  T i;
+  T common;
+  B () : i (2), common (200) {}
+};
+
+typedef B<int> Bint;
+
+class C : public virtual A<int>
+{
+public:
+  int i;
+  int c;
+  int common;
+  C () : i (3), c (30), common (300) {}
+};
+
+class BB : public A<int>
+{
+public:
+  int i;
+  BB () : i (20) {}
+};
+
+class CC : public A<int>
+{
+public:
+  int i;
+  CC () : i (30) {}
+};
+
+class Ambig : public BB, public CC
+{
+public:
+  int i;
+  Ambig () : i (1000) {}
+};
+
+class D : public Bint, public C
+{
+public:
+  int i;
+  int x;
+  Ambig am;
+  D () : i (4), x (40) {}
+
+#ifdef DEBUG
+#define SUM(X)					\
+  do						\
+    {						\
+      sum += (X);				\
+      printf ("" #X " = %d\n", (X));		\
+    }						\
+  while (0)
+#else
+#define SUM(X) sum += (X)
+#endif
+
+int
+f (void)
+  {
+    int sum = 0;
+
+    SUM (i);
+    SUM (D::i);
+    SUM (D::B<int>::i);
+    SUM (B<int>::i);
+    SUM (D::C::i);
+    SUM (C::i);
+    SUM (D::B<int>::A<int>::i);
+    SUM (B<int>::A<int>::i);
+    SUM (A<int>::i);
+    SUM (D::C::A<int>::i);
+    SUM (C::A<int>::i);
+    SUM (D::x);
+    SUM (x);
+    SUM (D::C::c);
+    SUM (C::c);
+    SUM (c);
+    SUM (D::A<int>::i);
+    SUM (Bint::i);
+    //SUM (D::Bint::i);
+    //SUM (D::Bint::A<int>::i);
+    SUM (Bint::A<int>::i);
+    // ambiguous: SUM (common);
+    SUM (B<int>::common);
+    SUM (C::common);
+    SUM (am.i);
+    // ambiguous: SUM (am.A<int>::i);
+
+    return sum;
+  }
+};
+
+int
+main (void)
+{
+  Bint b;
+  D d;
+
+  return d.f () + b.i;
+}
diff --git a/gdb/testsuite/gdb.cp/impl-this.exp b/gdb/testsuite/gdb.cp/impl-this.exp
new file mode 100644
index 0000000..ce7c780
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/impl-this.exp
@@ -0,1 +1,131 @@
+# Copyright 2013 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/>.
+
+# This file is part of the gdb testsuite
+
+# Test expressions which assume an implicit "this" with a qualified
+# name.
+
+if {[skip_cplus_tests]} { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+# First test expressions when there is no context.
+with_test_prefix "before run" {
+    gdb_test "print i" "No symbol \"i\" in current context."
+    gdb_test "print D::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::B<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print B<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::C::i" "Cannot reference non-static field \"i\""
+    gdb_test "print C::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::B<int>::A<int>::i" \
+	"Cannot reference non-static field \"i\""
+    gdb_test "print B<int>::A<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print A<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::C::A<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print C::A<int>::i" "Cannot reference non-static field \"i\""
+    gdb_test "print D::x" "Cannot reference non-static field \"x\""
+    gdb_test "print x" "No symbol \"x\" in current context."
+    gdb_test "print D::C::c" "Cannot reference non-static field \"c\""
+    gdb_test "print C::c" "Cannot reference non-static field \"c\""
+    gdb_test "print c" "No symbol \"c\" in current context."
+    gdb_test "print D::A<int>::i" "Cannot reference non-static field \"i\""
+}
+
+# Run to D::f.
+if {![runto_main]} {
+    perror "couldn't run to main"
+    continue
+}
+
+gdb_breakpoint "D::f"
+gdb_continue_to_breakpoint "continue to D::f"
+
+# Now test valid expressions in the class hierarchy for D.
+with_test_prefix "at D::f (valid expressions)" {
+    gdb_test "print i" "= 4"
+    gdb_test "print D::i" "= 4"
+    gdb_test "print D::B<int>::i" "= 2"
+    gdb_test "print B<int>::i" "= 2"
+    gdb_test "print D::Bint::i" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print Bint::i" "= 2"
+    gdb_test "print D::C::i" "= 3"
+    gdb_test "print C::i" "= 3"
+    gdb_test "print D::B<int>::A<int>::i" "= 1"
+    gdb_test "print B<int>::A<int>::i" "= 1"
+    gdb_test "print D::Bint::A<int>::i" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print Bint::A<int>::i" "= 1"
+    gdb_test "print A<int>::i" "= 1"
+    gdb_test "print D::C::A<int>::i" "= 1"
+    gdb_test "print C::A<int>::i" "= 1"
+    gdb_test "print D::x" "= 40"
+    gdb_test "print x" "= 40"
+    gdb_test "print D::C::c" "= 30"
+    gdb_test "print C::c" "= 30"
+    gdb_test "print c" "= 30"
+    gdb_test "print D::A<int>::i" "= 1"
+}
+
+# Test some invalid expressions
+with_test_prefix "at D::f (invalid expressions)" {
+    gdb_test "print D::B<int>::c" "There is no field named c"
+    gdb_test "print D::B<int>::A<int>::c" "There is no field named c"
+    gdb_test "print D::Bint::c" \
+	"No type \"Bint\" within class or namespace \"D\"."
+
+    gdb_test "print D::Bint::A<int>::c" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print D::C::A<int>::c" "There is no field named c"
+    gdb_test "print B<int>::c" "There is no field named c"
+    gdb_test "print B<int>::A<int>::c" "There is no field named c"
+    gdb_test "print Bint::c" "There is no field named c"
+    gdb_test "print Bint::A<int>::c" "There is no field named c"
+    gdb_test "print C::A<int>::c" "There is no field named c"
+    gdb_test "print D::B<int>::x" "There is no field named x"
+    gdb_test "print D::B<int>::A<int>::x" "There is no field named x"
+    gdb_test "print D::Bint::x" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print D::Bint::A<int>::x" \
+	"No type \"Bint\" within class or namespace \"D\"."
+    gdb_test "print B<int>::x" "There is no field named x"
+    gdb_test "print B<int>::A<int>::x" "There is no field named x"
+    gdb_test "print Bint::x" "There is no field named x"
+    gdb_test "print Bint::A<int>::x" "There is no field named x"
+    gdb_test "print D::C::x" "There is no field named x"
+    gdb_test "print C::x" "There is no field named x"
+    gdb_test "print D::C::A<int>::x" "There is no field named x"
+    gdb_test "print C::A<int>::x" "There is no field named x"
+}
+
+# Test some ambiguous names
+with_test_prefix "at D::f (ambiguous names)" {
+    gdb_test "print B<int>::common" " = 200"
+    gdb_test "print Bint::common" " = 200"
+    gdb_test "print C::common" " = 300"
+    gdb_test "print am.i" " = 1000"
+    gdb_test "print am.A<int>::i" \
+	"base class 'A<int>' is ambiguous in type 'Ambig'"
+    gdb_test "print am.BB::A<int>::i" \
+	"base class 'A<int>' is ambiguous in type 'Ambig'"
+    gdb_test "print am.CC::A<int>::i" \
+	"base class 'A<int>' is ambiguous in type 'Ambig'"
+}