Re: [RFC] logic change in m2-valprint.c

[Jim Blandy <jimb at codesourcery dot com>]

Mark Kettenis <mark.kettenis@xs4all.nl> writes:
>> Date: Mon, 2 Jul 2007 21:11:47 -0400
>> From: Daniel Jacobowitz <drow@false.org>
>> 
>> On Mon, Jul 02, 2007 at 05:05:12PM -0700, Michael Snyder wrote:
>> > is nowhere else for it to be set false), and there is no second
>> > time -- we will never enter this block again because we will set
>> > "element_seen" to true (and there is nowhere else for it to be
>> > set false again).
>> > 
>> > Was that clear?
>> 
>> Clear, but not right.
>> 
>>     152                   element_seen = 0;
>> 
>> On entry, empty_set = 1 and element_seen = 0.  We see an element,
>> which causes us set empty_set = 0 and element_seen = 1.  Then we see a
>> clear bit in the set and set element_seen to 0 and not change
>> empty_set.  Then we see a set bit, and element_seen == 0 with
>> empty_set == 0.  We print the comma.
>> 
>> I would test it, but I don't have an M-2 compiler and the expression
>> parser can't create sets.  Am I missing something?
>
> Dunno, but it is obvious that this bit of code is in need of a rewrite.

Yeah.  There's actually a nested loop there, but it's not easy to
see.  And the name 'empty_set' suggests something one can't know until
the traversal is complete...

I should stay away from 'beautiful code' contents, but is this any
better?

static void
m2_print_long_set (struct type *type, const gdb_byte *valaddr,
		   int embedded_offset, CORE_ADDR address,
		   struct ui_file *stream, int format,
		   enum val_prettyprint pretty)
{
  int field, len;
  LONGEST low_bound, high_bound;
  int printed_any = 0;
  struct type *target = NULL;

  /* We want to print runs of members using the '1..10' notation, but
     we don't know when a run has ended until we see a zero bit.  We
     set run_pending when we see what may be the start of a run; when
     run_pending is set, run_low and run_high record the known extent
     of the run.  */
  int run_pending = 0;
  LONGEST run_low = 0;
  LONGEST run_high = 0;

  CHECK_TYPEDEF (type);

  if (! get_long_set_bounds (type, &low_bound, &high_bound))
    {
      fprintf_filtered (stream, "{ %s }", _("<unknown bounds of set>"));
      return;
    }

  /* Walk the fields of the set, printing elements from each.  */
  fprintf_filtered (stream, "{");
  len = TYPE_NFIELDS (type);
  for (field = TYPE_N_BASECLASSES (type); field < len; field++)
    {
      struct type *field_range;
      LONGEST field_range_low, field_range_high;
      int i;

      field_range = TYPE_INDEX_TYPE (TYPE_FIELD_TYPE (type, field));
      if (get_discrete_bounds (field_range,
                               &field_range_low, &field_range_high)
          < 0)
        break;

      target = TYPE_TARGET_TYPE (field_range);
      if (target == NULL)
        target = builtin_type_int;
      
      for (i = field_range_low; i <= field_range_high; i++)
        {
          int bitval = value_bit_index (TYPE_FIELD_TYPE (type, field),
                                        (TYPE_FIELD_BITPOS (type, field) / 8) +
                                        valaddr + embedded_offset, i);
          if (bitval < 0)
            error (_("bit test is out of range"));
          else if (bitval > 0)
            {
              if (run_pending)
                /* Extend the existing run.  */
                run_high = i;
              else
                {
                  /* Start a new run.  */
                  run_pending = 1;
                  run_low = run_high = i;
                }

              /* We print things when we find the end of the run.  */
            }
          else
            {
              /* If we had a run going, we've now found its end, so
                 print it.  */
              if (run_pending)
                {
                  if (printed_any)
                    fprintf_filtered (stream, ", ");

                  print_type_scalar (target, run_low, stream);
                  if (run_low < run_high)
                    {
                      fprintf_filtered (stream, "..");
                      print_type_scalar (target, run_high, stream);
                    }

                  run_pending = 0;
                  printed_any = 1;
                }
            }
        }
    }

  /* Flush any pending range abutting the end of the set.  */
  if (run_pending)
    {
      if (printed_any)
        fprintf_filtered (stream, ", ");

      print_type_scalar (target, run_low, stream);
      if (run_low < run_high)
        {
          fprintf_filtered (stream, "..");
          print_type_scalar (target, run_high, stream);
        }
    }
                  
  fprintf_filtered (stream, "}");
}