This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Record and output access specifiers for nested typedefs


On 10/16/2017 12:56 PM, Simon Marchi wrote:
> On 2017-10-13 03:18 PM, Keith Seitz wrote:
>> gdb/ChangeLog:
>>
>> 	* c-typeprint.c (enum access_specifier): Moved here from
>> 	c_type_print_base.
>> 	(output_access_specifier): New function.
>> 	(c_type_print_base): Consider typedefs when assessing
>> 	whether access labels are needed.
>> 	Use output_access_specifier as needed.
>> 	Output access specifier for typedefs, if needed.
>> 	* dwarf2read.c (dwarf2_add_typedef): Record DW_AT_accessibility.
>> 	* gdbtypes.h (struct typedef_field) <is_protected, is_private>: New
>> 	fields.
>> 	(TYPE_TYPEDEF_FIELD_PROTECTED, TYPE_TYEPDEF_FIELD_PRIVATE): New
> "TYPE_TYEPDEF_FIELD_PRIVATE"
> 
Fixed.

>> 	accessor macros.
>>
>> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
>> index 978fcc4..3dc74f9 100644
>> --- a/gdb/c-typeprint.c
>> +++ b/gdb/c-typeprint.c
>> @@ -826,7 +836,42 @@ c_type_print_template_args (const struct type_print_options *flags,
>>      fputs_filtered (_("] "), stream);
>>  }
>>  
>> -/* Print the name of the type (or the ultimate pointer target,
>> +/* Output an access specifier to STREAM, if needed.  */
>> +
>> +static void
>> +output_access_specifier (struct ui_file *stream, enum access_specifier &access,
>> +			 int level, bool is_protected, bool is_private)
> I would suggest renaming access it to last_access or previous_access.  Could you also document
> it in the function comment?
Done.

> It might also be clearer to have output_access_specifier return the access_specifier,
> rather than modifying the passed value directly.  Otherwise, it's not really obvious
> what's happening in the caller.
Also done.

>> +  /* Print the name of the type (or the ultimate pointer target,
> Spurious indent change on that last line.
Removed.

>> diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp
>> index 256fa68..1f6d377 100644
>> --- a/gdb/testsuite/gdb.cp/classes.exp
>> +++ b/gdb/testsuite/gdb.cp/classes.exp
>> @@ -316,6 +316,64 @@ proc test_ptype_class_objects {} {
>>  	    { field  public "int y;" }
>>  	    { method public "DynamicBar(int, int);" }
>>  	}
>> +
>> +    # Classes with tyepdefs of different access.
> "tyepdefs".
Darn fingers. Fixed.

> 
>> +
>> +    cp_test_ptype_class \
>> +	"class class_with_typedefs" "" "class" "class_with_typedefs" \
>> +	{
>> +	    { field protected \
>> +		  "class_with_typedefs::public_int public_int_;" }
>> +	    { field protected \
>> +		  "class_with_typedefs::protected_int protected_int_;" }
>> +	    { field protected \
>> +		  "class_with_typedefs::private_int private_int_;" }
>> +	    { method public "class_with_typedefs(void);" }
>> +	    { method public "class_with_typedefs::public_int add_public(class_with_typedefs::public_int);" }
>> +	    { method public \
>> +		  "class_with_typedefs::public_int add_all(int);" }
>> +	    { method protected "class_with_typedefs::protected_int add_protected(class_with_typedefs::protected_int);" }
>> +	    { method private "class_with_typedefs::private_int add_private(class_with_typedefs::private_int);" }
>> +	    {typedef public "typedef int public_int;" }
>> +	    {typedef protected "typedef int protected_int;" }
>> +	    {typedef private "typedef int private_int;" }
> Add spaces after {, for consistency.
Also fixed. [How'd I not see that?!]

>> +    # For the following two cases, we cannot use cp_test_ptype_class.
>> +    # We need to explicitly check whether the access label was suppressed.
>> +    set ws {[\ \t\r\n]*}
> I don't think it's necessary to escape the space.
> 
Indeed not. Removed.

Thank you for the reivew.

Keith

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 978fcc4..970b6a0 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -32,6 +32,16 @@
 #include "cp-abi.h"
 #include "cp-support.h"
 
+/* A list of access specifiers used for printing.  */
+
+enum access_specifier
+{
+  s_none,
+  s_public,
+  s_private,
+  s_protected
+};
+
 static void c_type_print_varspec_prefix (struct type *,
 					 struct ui_file *,
 					 int, int, int,
@@ -826,6 +836,45 @@ c_type_print_template_args (const struct type_print_options *flags,
     fputs_filtered (_("] "), stream);
 }
 
+/* Output an access specifier to STREAM, if needed.  LAST_ACCESS is the
+   last access specifier output (typically returned by this function).  */
+
+static enum access_specifier
+output_access_specifier (struct ui_file *stream,
+			 enum access_specifier last_access,
+			 int level, bool is_protected, bool is_private)
+{
+  if (is_protected)
+    {
+      if (last_access != s_protected)
+	{
+	  last_access = s_protected;
+	  fprintfi_filtered (level + 2, stream,
+			     "protected:\n");
+	}
+    }
+  else if (is_private)
+    {
+      if (last_access != s_private)
+	{
+	  last_access = s_private;
+	  fprintfi_filtered (level + 2, stream,
+			     "private:\n");
+	}
+    }
+  else
+    {
+      if (last_access != s_public)
+	{
+	  last_access = s_public;
+	  fprintfi_filtered (level + 2, stream,
+			     "public:\n");
+	}
+    }
+
+  return last_access;
+}
+
 /* Print the name of the type (or the ultimate pointer target,
    function value or array element), or the description of a structure
    or union.
@@ -850,11 +899,7 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 {
   int i;
   int len, real_len;
-  enum
-    {
-      s_none, s_public, s_private, s_protected
-    }
-  section_type;
+  enum access_specifier section_type;
   int need_access_label = 0;
   int j, len2;
 
@@ -1034,6 +1079,18 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 			  break;
 		      }
 		  }
+		QUIT;
+		if (!need_access_label)
+		  {
+		    for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i)
+		      {
+			if (!TYPE_TYPEDEF_FIELD_PRIVATE (type, i))
+			  {
+			    need_access_label = 1;
+			    break;
+			  }
+		      }
+		  }
 	      }
 	    else
 	      {
@@ -1068,6 +1125,19 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 			  break;
 		      }
 		  }
+		QUIT;
+		if (!need_access_label)
+		  {
+		    for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i)
+		      {
+			if (TYPE_TYPEDEF_FIELD_PROTECTED (type, i)
+			    || TYPE_TYPEDEF_FIELD_PRIVATE (type, i))
+			  {
+			    need_access_label = 1;
+			    break;
+			  }
+		      }
+		  }
 	      }
 
 	    /* If there is a base class for this type,
@@ -1088,33 +1158,10 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 
 		if (need_access_label)
 		  {
-		    if (TYPE_FIELD_PROTECTED (type, i))
-		      {
-			if (section_type != s_protected)
-			  {
-			    section_type = s_protected;
-			    fprintfi_filtered (level + 2, stream,
-					       "protected:\n");
-			  }
-		      }
-		    else if (TYPE_FIELD_PRIVATE (type, i))
-		      {
-			if (section_type != s_private)
-			  {
-			    section_type = s_private;
-			    fprintfi_filtered (level + 2, stream,
-					       "private:\n");
-			  }
-		      }
-		    else
-		      {
-			if (section_type != s_public)
-			  {
-			    section_type = s_public;
-			    fprintfi_filtered (level + 2, stream,
-					       "public:\n");
-			  }
-		      }
+		    section_type = output_access_specifier
+		      (stream, section_type, level,
+		       TYPE_FIELD_PROTECTED (type, i),
+		       TYPE_FIELD_PRIVATE (type, i));
 		  }
 
 		print_spaces_filtered (level + 4, stream);
@@ -1188,33 +1235,10 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 		  inner_cleanup = make_cleanup (null_cleanup, NULL);
 
 		  QUIT;
-		  if (TYPE_FN_FIELD_PROTECTED (f, j))
-		    {
-		      if (section_type != s_protected)
-			{
-			  section_type = s_protected;
-			  fprintfi_filtered (level + 2, stream,
-					     "protected:\n");
-			}
-		    }
-		  else if (TYPE_FN_FIELD_PRIVATE (f, j))
-		    {
-		      if (section_type != s_private)
-			{
-			  section_type = s_private;
-			  fprintfi_filtered (level + 2, stream,
-					     "private:\n");
-			}
-		    }
-		  else
-		    {
-		      if (section_type != s_public)
-			{
-			  section_type = s_public;
-			  fprintfi_filtered (level + 2, stream,
-					     "public:\n");
-			}
-		    }
+		  section_type = output_access_specifier
+		    (stream, section_type, level,
+		     TYPE_FN_FIELD_PROTECTED (f, j),
+		     TYPE_FN_FIELD_PRIVATE (f, j));
 
 		  print_spaces_filtered (level + 4, stream);
 		  if (TYPE_FN_FIELD_VIRTUAL_P (f, j))
@@ -1326,6 +1350,13 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 		  gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF);
 		  target = TYPE_TARGET_TYPE (target);
 
+		  if (need_access_label)
+		    {
+		      section_type = output_access_specifier
+			(stream, section_type, level,
+			 TYPE_TYPEDEF_FIELD_PROTECTED (type, i),
+			 TYPE_TYPEDEF_FIELD_PRIVATE (type, i));
+		    }
 		  print_spaces_filtered (level + 4, stream);
 		  fprintf_filtered (stream, "typedef ");
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 9bdb75d..c0200f9 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13274,6 +13274,28 @@ dwarf2_add_typedef (struct field_info *fip, struct die_info *die,
 
   fp->type = read_type_die (die, cu);
 
+  /* Save accessibility.  */
+  enum dwarf_access_attribute accessibility;
+  struct attribute *attr = dwarf2_attr (die, DW_AT_accessibility, cu);
+  if (attr != NULL)
+    accessibility = (enum dwarf_access_attribute) DW_UNSND (attr);
+  else
+    accessibility = dwarf2_default_access_attribute (die, cu);
+  switch (accessibility)
+    {
+    case DW_ACCESS_public:
+      /* The assumed value if neither private nor protected.  */
+      break;
+    case DW_ACCESS_private:
+      fp->is_private = 1;
+      break;
+    case DW_ACCESS_protected:
+      fp->is_protected = 1;
+      break;
+    default:
+      gdb_assert_not_reached ("unexpected accessibility attribute");
+    }
+
   new_field->next = fip->typedef_field_list;
   fip->typedef_field_list = new_field;
   fip->typedef_field_list_count++;
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 0e0aead..48af6f1 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -896,6 +896,12 @@ struct typedef_field
   /* * Type this typedef named NAME represents.  */
 
   struct type *type;
+
+  /* * True if this field was declared protected, false otherwise.  */
+  unsigned int is_protected : 1;
+
+  /* * True if this field was declared private, false otherwise.  */
+  unsigned int is_private : 1;
 };
 
 /* * C++ language-specific information for TYPE_CODE_STRUCT and
@@ -1427,6 +1433,7 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
 #define TYPE_FN_FIELD_VIRTUAL_P(thisfn, n) ((thisfn)[n].voffset > 1)
 #define TYPE_FN_FIELD_STATIC_P(thisfn, n) ((thisfn)[n].voffset == VOFFSET_STATIC)
 
+/* Accessors for typedefs defined by a class.  */
 #define TYPE_TYPEDEF_FIELD_ARRAY(thistype) \
   TYPE_CPLUS_SPECIFIC (thistype)->typedef_field
 #define TYPE_TYPEDEF_FIELD(thistype, n) \
@@ -1437,6 +1444,10 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
   TYPE_TYPEDEF_FIELD (thistype, n).type
 #define TYPE_TYPEDEF_FIELD_COUNT(thistype) \
   TYPE_CPLUS_SPECIFIC (thistype)->typedef_field_count
+#define TYPE_TYPEDEF_FIELD_PROTECTED(thistype, n) \
+  TYPE_TYPEDEF_FIELD (thistype, n).is_protected
+#define TYPE_TYPEDEF_FIELD_PRIVATE(thistype, n)        \
+  TYPE_TYPEDEF_FIELD (thistype, n).is_private
 
 #define TYPE_IS_OPAQUE(thistype) \
   (((TYPE_CODE (thistype) == TYPE_CODE_STRUCT) \
diff --git a/gdb/testsuite/gdb.cp/classes.cc b/gdb/testsuite/gdb.cp/classes.cc
index 2a81473..50f0740 100644
--- a/gdb/testsuite/gdb.cp/classes.cc
+++ b/gdb/testsuite/gdb.cp/classes.cc
@@ -545,6 +545,82 @@ small::method ()
   return x + 5;
 }
 
+class class_with_typedefs
+{
+public:
+  typedef int public_int;
+protected:
+  typedef int protected_int;
+private:
+  typedef int private_int;
+
+public:
+  class_with_typedefs ()
+    : public_int_ (1), protected_int_ (2), private_int_ (3) {}
+  public_int add_public (public_int a) { return a + public_int_; }
+  public_int add_all (int a)
+  { return add_public (a) + add_protected (a) + add_private (a); }
+
+protected:
+  protected_int add_protected (protected_int a) { return a + protected_int_; }
+
+private:
+  private_int add_private (private_int a) { return a + private_int_; }
+
+protected:
+  public_int public_int_;
+  protected_int protected_int_;
+  private_int private_int_;
+};
+
+class class_with_public_typedef
+{
+  int a;
+public:
+  typedef int INT;
+  INT b;
+};
+
+class class_with_protected_typedef
+{
+  int a;
+protected:
+  typedef int INT;
+  INT b;
+};
+
+class class_with_private_typedef
+{
+  int a;
+private:
+  typedef int INT;
+  INT b;
+};
+
+struct struct_with_public_typedef
+{
+  int a;
+public:
+  typedef int INT;
+  INT b;
+};
+
+struct struct_with_protected_typedef
+{
+  int a;
+protected:
+  typedef int INT;
+  INT b;
+};
+
+struct struct_with_private_typedef
+{
+  int a;
+private:
+  typedef int INT;
+  INT b;
+};
+
 void marker_reg1 () {}
 
 int
@@ -624,3 +700,10 @@ protected_class protected_c;
 default_private_class default_private_c;
 explicit_private_class explicit_private_c;
 mixed_protection_class mixed_protection_c;
+class_with_typedefs class_with_typedefs_c;
+class_with_public_typedef class_with_public_typedef_c;
+class_with_protected_typedef class_with_protected_typedef_c;
+class_with_private_typedef class_with_private_typedef_c;
+struct_with_public_typedef struct_with_public_typedef_s;
+struct_with_protected_typedef struct_with_protected_typedef_s;
+struct_with_private_typedef struct_with_private_typedef_s;
diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp
index 256fa68..9e2630a 100644
--- a/gdb/testsuite/gdb.cp/classes.exp
+++ b/gdb/testsuite/gdb.cp/classes.exp
@@ -316,6 +316,64 @@ proc test_ptype_class_objects {} {
 	    { field  public "int y;" }
 	    { method public "DynamicBar(int, int);" }
 	}
+
+    # Classes with typedefs of different access.
+
+    cp_test_ptype_class \
+	"class class_with_typedefs" "" "class" "class_with_typedefs" \
+	{
+	    { field protected \
+		  "class_with_typedefs::public_int public_int_;" }
+	    { field protected \
+		  "class_with_typedefs::protected_int protected_int_;" }
+	    { field protected \
+		  "class_with_typedefs::private_int private_int_;" }
+	    { method public "class_with_typedefs(void);" }
+	    { method public "class_with_typedefs::public_int add_public(class_with_typedefs::public_int);" }
+	    { method public \
+		  "class_with_typedefs::public_int add_all(int);" }
+	    { method protected "class_with_typedefs::protected_int add_protected(class_with_typedefs::protected_int);" }
+	    { method private "class_with_typedefs::private_int add_private(class_with_typedefs::private_int);" }
+	    { typedef public "typedef int public_int;" }
+	    { typedef protected "typedef int protected_int;" }
+	    { typedef private "typedef int private_int;" }
+	}
+
+    cp_test_ptype_class \
+	"class class_with_public_typedef" "" "class" \
+	"class_with_public_typedef" {
+	    { field private "int a;" }
+	    { field public "class_with_public_typedef::INT b;" }
+	    { typedef public "typedef int INT;" }
+	}
+    cp_test_ptype_class \
+	"class class_with_protected_typedef" "" "class" \
+	"class_with_protected_typedef" {
+	    { field private "int a;" }
+	    { field protected "class_with_protected_typedef::INT b;" }
+	    { typedef protected "typedef int INT;" }
+	}
+    cp_test_ptype_class \
+	"struct struct_with_protected_typedef" "" "struct" \
+	"struct_with_protected_typedef" {
+	    { field public "int a;" }
+	    { field protected "struct_with_protected_typedef::INT b;" }
+	    { typedef protected "typedef int INT;" }
+	}
+    cp_test_ptype_class \
+	"struct struct_with_private_typedef" "" "struct" \
+	"struct_with_private_typedef" {
+	    { field public "int a;" }
+	    { field private "struct_with_private_typedef::INT b;" }
+	    { typedef private "typedef int INT;" }
+	}
+
+    # For the following two cases, we cannot use cp_test_ptype_class.
+    # We need to explicitly check whether the access label was suppressed.
+    set ws {[ \t\r\n]*}
+    foreach {tag lbl} {"class" "private" "struct" "public"} {
+	gdb_test "ptype/r ${tag}_with_${lbl}_typedef" "type = $tag ${tag}_with_${lbl}_typedef \{${ws}int a;${ws}${tag}_with_${lbl}_typedef::INT b;${ws}typedef int INT;${ws}\}"
+    }
 }
 
 # Test simple access to class members.
-- 
2.1.0


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]