public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA] Add inclusive range support for Rust
@ 2018-03-29 20:16 Tom Tromey
  2018-04-17 19:48 ` Tom Tromey
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Tom Tromey @ 2018-03-29 20:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Rust recently stabilized the inclusive range feature:

    https://github.com/rust-lang/rust/issues/28237

An inclusive range is an expression like "..= EXPR" or "EXPR ..=
EXPR".  It is like an ordinary range, except the upper bound is
inclusive, not exclusive.

This patch adds support for this feature to gdb.

Regression tested on x86-64 Fedora 26.

Note that review is required because this patch touches some non-Rust
code.

2018-03-29  Tom Tromey  <tom@tromey.com>

	PR rust/22545:
	* rust-lang.c (rust_inclusive_range_type_p): New function.
	(rust_range): Handle inclusive ranges.
	(rust_compute_range): Likewise.
	* rust-exp.y (struct rust_op) <inclusive>: New field.
	(DOTDOTEQ): New constant.
	(range_expr): Add "..=" productions.
	(operator_tokens): Add "..=" token.
	(ast_range): Add "inclusive" parameter.
	(convert_ast_to_expression) <case OP_RANGE>: Handle inclusive
	ranges.
	* parse.c (operator_length_standard) <case OP_RANGE>: Handle new
	bounds values.
	* expression.h (enum range_type) <NONE_BOUND_DEFAULT_INCLUSIVE,
	LOW_BOUND_DEFAULT_INCLUSIVE>: New constants.
	* expprint.c (print_subexp_standard): Handle new bounds values.
	(dump_subexp_body_standard): Likewise.

2018-03-29  Tom Tromey  <tom@tromey.com>

	PR rust/22545:
	* gdb.rust/simple.exp: Add inclusive range tests.
---
 gdb/ChangeLog                     | 20 ++++++++++++++++++++
 gdb/expprint.c                    | 15 ++++++++++++++-
 gdb/expression.h                  | 11 +++++++----
 gdb/parse.c                       |  2 ++
 gdb/rust-exp.y                    | 39 ++++++++++++++++++++++++++++++---------
 gdb/rust-lang.c                   | 27 +++++++++++++++++++++++----
 gdb/testsuite/ChangeLog           |  5 +++++
 gdb/testsuite/gdb.rust/simple.exp |  6 ++++++
 8 files changed, 107 insertions(+), 18 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 78f427fb7e..a42cf6bb8e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,23 @@
+2018-03-29  Tom Tromey  <tom@tromey.com>
+
+	PR rust/22545:
+	* rust-lang.c (rust_inclusive_range_type_p): New function.
+	(rust_range): Handle inclusive ranges.
+	(rust_compute_range): Likewise.
+	* rust-exp.y (struct rust_op) <inclusive>: New field.
+	(DOTDOTEQ): New constant.
+	(range_expr): Add "..=" productions.
+	(operator_tokens): Add "..=" token.
+	(ast_range): Add "inclusive" parameter.
+	(convert_ast_to_expression) <case OP_RANGE>: Handle inclusive
+	ranges.
+	* parse.c (operator_length_standard) <case OP_RANGE>: Handle new
+	bounds values.
+	* expression.h (enum range_type) <NONE_BOUND_DEFAULT_INCLUSIVE,
+	LOW_BOUND_DEFAULT_INCLUSIVE>: New constants.
+	* expprint.c (print_subexp_standard): Handle new bounds values.
+	(dump_subexp_body_standard): Likewise.
+
 2018-03-27  Tom Tromey  <tom@tromey.com>
 
 	* utils.c (prompt_for_continue): Use unique_xmalloc_ptr.
diff --git a/gdb/expprint.c b/gdb/expprint.c
index 9d1884f290..344dddb1a6 100644
--- a/gdb/expprint.c
+++ b/gdb/expprint.c
@@ -583,9 +583,16 @@ print_subexp_standard (struct expression *exp, int *pos,
 
 	fputs_filtered ("RANGE(", stream);
 	if (range_type == HIGH_BOUND_DEFAULT
-	    || range_type == NONE_BOUND_DEFAULT)
+	    || range_type == NONE_BOUND_DEFAULT
+	    || range_type == NONE_BOUND_DEFAULT_INCLUSIVE)
 	  print_subexp (exp, pos, stream, PREC_ABOVE_COMMA);
 	fputs_filtered ("..", stream);
+	if (range_type == NONE_BOUND_DEFAULT_INCLUSIVE
+	    || range_type == LOW_BOUND_DEFAULT_INCLUSIVE)
+	  {
+	    /* Rust-style syntax.  */
+	    fputs_filtered ("=", stream);
+	  }
 	if (range_type == LOW_BOUND_DEFAULT
 	    || range_type == NONE_BOUND_DEFAULT)
 	  print_subexp (exp, pos, stream, PREC_ABOVE_COMMA);
@@ -1102,12 +1109,18 @@ dump_subexp_body_standard (struct expression *exp,
 	  case LOW_BOUND_DEFAULT:
 	    fputs_filtered ("Range '..EXP'", stream);
 	    break;
+	  case LOW_BOUND_DEFAULT_INCLUSIVE:
+	    fputs_filtered ("Range '..=EXP'", stream);
+	    break;
 	  case HIGH_BOUND_DEFAULT:
 	    fputs_filtered ("Range 'EXP..'", stream);
 	    break;
 	  case NONE_BOUND_DEFAULT:
 	    fputs_filtered ("Range 'EXP..EXP'", stream);
 	    break;
+	  case NONE_BOUND_DEFAULT_INCLUSIVE:
+	    fputs_filtered ("Range 'EXP..=EXP'", stream);
+	    break;
 	  default:
 	    fputs_filtered ("Invalid Range!", stream);
 	    break;
diff --git a/gdb/expression.h b/gdb/expression.h
index 7abd7f7503..86ee698aed 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -150,15 +150,18 @@ extern void dump_prefix_expression (struct expression *, struct ui_file *);
 
 /* In an OP_RANGE expression, either bound could be empty, indicating
    that its value is by default that of the corresponding bound of the
-   array or string.  So we have four sorts of subrange.  This
-   enumeration type is to identify this.  */
-   
+   array or string.  Also, the upper end of the range can be exclusive
+   or inclusive.  So we have six sorts of subrange.  This enumeration
+   type is to identify this.  */
+
 enum range_type
   {
     BOTH_BOUND_DEFAULT,		/* "(:)"  */
     LOW_BOUND_DEFAULT,		/* "(:high)"  */
     HIGH_BOUND_DEFAULT,		/* "(low:)"  */
-    NONE_BOUND_DEFAULT		/* "(low:high)"  */
+    NONE_BOUND_DEFAULT,		/* "(low:high)"  */
+    NONE_BOUND_DEFAULT_INCLUSIVE, /* Rust "low..=high"  */
+    LOW_BOUND_DEFAULT_INCLUSIVE, /* Rust "..=high"  */
   };
 
 #endif /* !defined (EXPRESSION_H) */
diff --git a/gdb/parse.c b/gdb/parse.c
index 3abb9d4219..77915cc0bc 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -1002,6 +1002,7 @@ operator_length_standard (const struct expression *expr, int endpos,
       switch (range_type)
 	{
 	case LOW_BOUND_DEFAULT:
+	case LOW_BOUND_DEFAULT_INCLUSIVE:
 	case HIGH_BOUND_DEFAULT:
 	  args = 1;
 	  break;
@@ -1009,6 +1010,7 @@ operator_length_standard (const struct expression *expr, int endpos,
 	  args = 0;
 	  break;
 	case NONE_BOUND_DEFAULT:
+	case NONE_BOUND_DEFAULT_INCLUSIVE:
 	  args = 2;
 	  break;
 	}
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index b661a803e3..cea6df1dc3 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -111,7 +111,8 @@ static const struct rust_op *ast_string (struct stoken str);
 static const struct rust_op *ast_struct (const struct rust_op *name,
 					 rust_set_vector *fields);
 static const struct rust_op *ast_range (const struct rust_op *lhs,
-					const struct rust_op *rhs);
+					const struct rust_op *rhs,
+					bool inclusive);
 static const struct rust_op *ast_array_type (const struct rust_op *lhs,
 					     struct typed_val_int val);
 static const struct rust_op *ast_slice_type (const struct rust_op *type);
@@ -300,6 +301,9 @@ struct rust_op
      name occurred at the end of the expression and is eligible for
      completion.  */
   unsigned int completing : 1;
+  /* For OP_RANGE, indicates whether the range is inclusive or
+     exclusive.  */
+  unsigned int inclusive : 1;
   /* Operands of expression.  Which one is used and how depends on the
      particular opcode.  */
   RUSTSTYPE left;
@@ -333,6 +337,7 @@ struct rust_op
 
 /* Operator tokens.  */
 %token <voidval> DOTDOT
+%token <voidval> DOTDOTEQ
 %token <voidval> OROR
 %token <voidval> ANDAND
 %token <voidval> EQEQ
@@ -382,7 +387,7 @@ struct rust_op
 %type <one_field_init> struct_expr_tail
 
 /* Precedence.  */
-%nonassoc DOTDOT
+%nonassoc DOTDOT DOTDOTEQ
 %right '=' COMPOUND_ASSIGN
 %left OROR
 %left ANDAND
@@ -535,13 +540,17 @@ array_expr:
 
 range_expr:
 	expr DOTDOT
-		{ $$ = ast_range ($1, NULL); }
+		{ $$ = ast_range ($1, NULL, false); }
 |	expr DOTDOT expr
-		{ $$ = ast_range ($1, $3); }
+		{ $$ = ast_range ($1, $3, false); }
+|	expr DOTDOTEQ expr
+		{ $$ = ast_range ($1, $3, true); }
 |	DOTDOT expr
-		{ $$ = ast_range (NULL, $2); }
+		{ $$ = ast_range (NULL, $2, false); }
+|	DOTDOTEQ expr
+		{ $$ = ast_range (NULL, $2, true); }
 |	DOTDOT
-		{ $$ = ast_range (NULL, NULL); }
+		{ $$ = ast_range (NULL, NULL, false); }
 ;
 
 literal:
@@ -956,6 +965,7 @@ static const struct token_info operator_tokens[] =
   { "&=", COMPOUND_ASSIGN, BINOP_BITWISE_AND },
   { "|=", COMPOUND_ASSIGN, BINOP_BITWISE_IOR },
   { "^=", COMPOUND_ASSIGN, BINOP_BITWISE_XOR },
+  { "..=", DOTDOTEQ, OP_NULL },
 
   { "::", COLONCOLON, OP_NULL },
   { "..", DOTDOT, OP_NULL },
@@ -1841,11 +1851,13 @@ ast_structop_anonymous (const struct rust_op *left,
 /* Make a range operation.  */
 
 static const struct rust_op *
-ast_range (const struct rust_op *lhs, const struct rust_op *rhs)
+ast_range (const struct rust_op *lhs, const struct rust_op *rhs,
+	   bool inclusive)
 {
   struct rust_op *result = OBSTACK_ZALLOC (work_obstack, struct rust_op);
 
   result->opcode = OP_RANGE;
+  result->inclusive = inclusive;
   result->left.op = lhs;
   result->right.op = rhs;
 
@@ -2473,13 +2485,22 @@ convert_ast_to_expression (struct parser_state *state,
 	  {
 	    convert_ast_to_expression (state, operation->right.op, top);
 	    if (kind == BOTH_BOUND_DEFAULT)
-	      kind = LOW_BOUND_DEFAULT;
+	      kind = (operation->inclusive
+		      ? LOW_BOUND_DEFAULT_INCLUSIVE : LOW_BOUND_DEFAULT);
 	    else
 	      {
 		gdb_assert (kind == HIGH_BOUND_DEFAULT);
-		kind = NONE_BOUND_DEFAULT;
+		kind = (operation->inclusive
+			? NONE_BOUND_DEFAULT_INCLUSIVE : NONE_BOUND_DEFAULT);
 	      }
 	  }
+	else
+	  {
+	    /* Nothing should make an inclusive range without an upper
+	       bound.  */
+	    gdb_assert (!operation->inclusive);
+	  }
+
 	write_exp_elt_opcode (state, OP_RANGE);
 	write_exp_elt_longcst (state, kind);
 	write_exp_elt_opcode (state, OP_RANGE);
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 6e0537f358..6174f3be61 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -180,6 +180,17 @@ rust_range_type_p (struct type *type)
   return strcmp (TYPE_FIELD_NAME (type, i), "end") == 0;
 }
 
+/* Return true if TYPE is an inclusive range type, otherwise false.
+   This is only valid for types which are already known to be range
+   types.  */
+
+static bool
+rust_inclusive_range_type_p (struct type *type)
+{
+  return (strstr (TYPE_TAG_NAME (type), "::RangeInclusive") != NULL
+	  || strstr (TYPE_TAG_NAME (type), "::RangeToInclusive") != NULL);
+}
+
 /* Return true if TYPE seems to be the type "u8", otherwise false.  */
 
 static bool
@@ -1150,10 +1161,14 @@ rust_range (struct expression *exp, int *pos, enum noside noside)
   kind = (enum range_type) longest_to_int (exp->elts[*pos + 1].longconst);
   *pos += 3;
 
-  if (kind == HIGH_BOUND_DEFAULT || kind == NONE_BOUND_DEFAULT)
+  if (kind == HIGH_BOUND_DEFAULT || kind == NONE_BOUND_DEFAULT
+      || kind == NONE_BOUND_DEFAULT_INCLUSIVE)
     low = evaluate_subexp (NULL_TYPE, exp, pos, noside);
-  if (kind == LOW_BOUND_DEFAULT || kind == NONE_BOUND_DEFAULT)
+  if (kind == LOW_BOUND_DEFAULT || kind == LOW_BOUND_DEFAULT_INCLUSIVE
+      || kind == NONE_BOUND_DEFAULT || kind == NONE_BOUND_DEFAULT_INCLUSIVE)
     high = evaluate_subexp (NULL_TYPE, exp, pos, noside);
+  bool inclusive = (kind == NONE_BOUND_DEFAULT_INCLUSIVE
+		    || kind == LOW_BOUND_DEFAULT_INCLUSIVE);
 
   if (noside == EVAL_SKIP)
     return value_from_longest (builtin_type (exp->gdbarch)->builtin_int, 1);
@@ -1168,7 +1183,8 @@ rust_range (struct expression *exp, int *pos, enum noside noside)
       else
 	{
 	  index_type = value_type (high);
-	  name = "std::ops::RangeTo";
+	  name = (inclusive
+		  ? "std::ops::RangeToInclusive" : "std::ops::RangeTo");
 	}
     }
   else
@@ -1183,7 +1199,7 @@ rust_range (struct expression *exp, int *pos, enum noside noside)
 	  if (!types_equal (value_type (low), value_type (high)))
 	    error (_("Range expression with different types"));
 	  index_type = value_type (low);
-	  name = "std::ops::Range";
+	  name = inclusive ? "std::ops::RangeInclusive" : "std::ops::Range";
 	}
     }
 
@@ -1259,6 +1275,9 @@ rust_compute_range (struct type *type, struct value *range,
       *kind = (*kind == BOTH_BOUND_DEFAULT
 	       ? LOW_BOUND_DEFAULT : NONE_BOUND_DEFAULT);
       *high = value_as_long (value_field (range, i));
+
+      if (rust_inclusive_range_type_p (type))
+	++*high;
     }
 }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 806744dd45..a4fbdf5dec 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-03-29  Tom Tromey  <tom@tromey.com>
+
+	PR rust/22545:
+	* gdb.rust/simple.exp: Add inclusive range tests.
+
 2018-03-27  Joel Brobecker  <brobecker@adacore.com>
 
 	* gdb.ada/varsize_limit: New testcase.
diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
index 2db596b932..8908af372b 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -216,7 +216,9 @@ gdb_test "print r###\"###hello\"##" "Unexpected EOF in string"
 gdb_test "print r###\"hello###" "Unexpected EOF in string"
 
 gdb_test "print 0..5" " = .*::ops::Range.* \\{start: 0, end: 5\\}"
+gdb_test "print 0..=5" " = .*::ops::RangeInclusive.* \\{start: 0, end: 5\\}"
 gdb_test "print ..5" " = .*::ops::RangeTo.* \\{end: 5\\}"
+gdb_test "print ..=5" " = .*::ops::RangeToInclusive.* \\{end: 5\\}"
 gdb_test "print 5.." " = .*::ops::RangeFrom.* \\{start: 5\\}"
 gdb_test "print .." " = .*::ops::RangeFull"
 
@@ -241,7 +243,9 @@ proc test_one_slice {svar length base range} {
 }
 
 test_one_slice slice 1 w 2..3
+test_one_slice slice 1 w 2..=2
 test_one_slice slice2 1 slice 0..1
+test_one_slice slice2 1 slice 0..=0
 
 test_one_slice all1 4 w ..
 test_one_slice all2 1 slice ..
@@ -250,7 +254,9 @@ test_one_slice from1 3 w 1..
 test_one_slice from2 0 slice 1..
 
 test_one_slice to1 3 w ..3
+test_one_slice to1 3 w ..=2
 test_one_slice to2 1 slice ..1
+test_one_slice to2 1 slice ..=0
 
 gdb_test "print w\[2..3\]" "Can't take slice of array without '&'"
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Add inclusive range support for Rust
  2018-03-29 20:16 [RFA] Add inclusive range support for Rust Tom Tromey
@ 2018-04-17 19:48 ` Tom Tromey
  2018-04-25 15:33   ` Tom Tromey
  2018-04-25 16:27 ` Joel Brobecker
  2018-04-25 16:52 ` Joel Brobecker
  2 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2018-04-17 19:48 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> Rust recently stabilized the inclusive range feature:
Tom>     https://github.com/rust-lang/rust/issues/28237
[...]

Tom> Note that review is required because this patch touches some non-Rust
Tom> code.

Ping.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Add inclusive range support for Rust
  2018-04-17 19:48 ` Tom Tromey
@ 2018-04-25 15:33   ` Tom Tromey
  2018-04-25 16:04     ` Joel Brobecker
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2018-04-25 15:33 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> Rust recently stabilized the inclusive range feature:
Tom> https://github.com/rust-lang/rust/issues/28237
Tom> [...]

Tom> Note that review is required because this patch touches some non-Rust
Tom> code.

Tom> Ping.

Ping.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Add inclusive range support for Rust
  2018-04-25 15:33   ` Tom Tromey
@ 2018-04-25 16:04     ` Joel Brobecker
  0 siblings, 0 replies; 9+ messages in thread
From: Joel Brobecker @ 2018-04-25 16:04 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> Tom> Ping.
> 
> Ping.

Sorry for the delay, Tom. I'm looking into it now.

-- 
Joel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Add inclusive range support for Rust
  2018-03-29 20:16 [RFA] Add inclusive range support for Rust Tom Tromey
  2018-04-17 19:48 ` Tom Tromey
@ 2018-04-25 16:27 ` Joel Brobecker
  2018-04-26 19:51   ` Tom Tromey
  2018-04-25 16:52 ` Joel Brobecker
  2 siblings, 1 reply; 9+ messages in thread
From: Joel Brobecker @ 2018-04-25 16:27 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Hi Tom,

>  /* In an OP_RANGE expression, either bound could be empty, indicating
>     that its value is by default that of the corresponding bound of the
> -   array or string.  So we have four sorts of subrange.  This
> -   enumeration type is to identify this.  */
> -   
> +   array or string.  Also, the upper end of the range can be exclusive
> +   or inclusive.  So we have six sorts of subrange.  This enumeration
> +   type is to identify this.  */
> +
>  enum range_type
>    {
>      BOTH_BOUND_DEFAULT,		/* "(:)"  */
>      LOW_BOUND_DEFAULT,		/* "(:high)"  */
>      HIGH_BOUND_DEFAULT,		/* "(low:)"  */
> -    NONE_BOUND_DEFAULT		/* "(low:high)"  */
> +    NONE_BOUND_DEFAULT,		/* "(low:high)"  */
> +    NONE_BOUND_DEFAULT_INCLUSIVE, /* Rust "low..=high"  */
> +    LOW_BOUND_DEFAULT_INCLUSIVE, /* Rust "..=high"  */
>    };

Where the bounds exclusive before? The comments and the samples
of code I have been finding seem to indicate that the bounds
were already considered inclusive. But I can see how this is
not all that clear.

Perhaps one way to clarify that is to use language-agnostic mathematical
notations for the ranges? Eg, using square brackets such as "[1:3[" or
perhaps "[1:3)" as I have sometimes seen?

I'll continue with the rest of the patch in the meantime...

-- 
Joel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Add inclusive range support for Rust
  2018-03-29 20:16 [RFA] Add inclusive range support for Rust Tom Tromey
  2018-04-17 19:48 ` Tom Tromey
  2018-04-25 16:27 ` Joel Brobecker
@ 2018-04-25 16:52 ` Joel Brobecker
  2018-04-26 20:16   ` Tom Tromey
  2 siblings, 1 reply; 9+ messages in thread
From: Joel Brobecker @ 2018-04-25 16:52 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Thu, Mar 29, 2018 at 02:16:09PM -0600, Tom Tromey wrote:
> Rust recently stabilized the inclusive range feature:
> 
>     https://github.com/rust-lang/rust/issues/28237
> 
> An inclusive range is an expression like "..= EXPR" or "EXPR ..=
> EXPR".  It is like an ordinary range, except the upper bound is
> inclusive, not exclusive.
> 
> This patch adds support for this feature to gdb.
> 
> Regression tested on x86-64 Fedora 26.
> 
> Note that review is required because this patch touches some non-Rust
> code.
> 
> 2018-03-29  Tom Tromey  <tom@tromey.com>
> 
> 	PR rust/22545:
> 	* rust-lang.c (rust_inclusive_range_type_p): New function.
> 	(rust_range): Handle inclusive ranges.
> 	(rust_compute_range): Likewise.
> 	* rust-exp.y (struct rust_op) <inclusive>: New field.
> 	(DOTDOTEQ): New constant.
> 	(range_expr): Add "..=" productions.
> 	(operator_tokens): Add "..=" token.
> 	(ast_range): Add "inclusive" parameter.
> 	(convert_ast_to_expression) <case OP_RANGE>: Handle inclusive
> 	ranges.
> 	* parse.c (operator_length_standard) <case OP_RANGE>: Handle new
> 	bounds values.
> 	* expression.h (enum range_type) <NONE_BOUND_DEFAULT_INCLUSIVE,
> 	LOW_BOUND_DEFAULT_INCLUSIVE>: New constants.
> 	* expprint.c (print_subexp_standard): Handle new bounds values.
> 	(dump_subexp_body_standard): Likewise.

I'm not sure I'm competent to review, but once I understand better
the existing enums for enum range_type, I think I'll be able to
officially approve.

A couple of comments below.

> @@ -1102,12 +1109,18 @@ dump_subexp_body_standard (struct expression *exp,
>  	  case LOW_BOUND_DEFAULT:
>  	    fputs_filtered ("Range '..EXP'", stream);
>  	    break;
> +	  case LOW_BOUND_DEFAULT_INCLUSIVE:
> +	    fputs_filtered ("Range '..=EXP'", stream);
> +	    break;
>  	  case HIGH_BOUND_DEFAULT:
>  	    fputs_filtered ("Range 'EXP..'", stream);
>  	    break;
>  	  case NONE_BOUND_DEFAULT:
>  	    fputs_filtered ("Range 'EXP..EXP'", stream);
>  	    break;
> +	  case NONE_BOUND_DEFAULT_INCLUSIVE:
> +	    fputs_filtered ("Range 'EXP..=EXP'", stream);
> +	    break;
>  	  default:
>  	    fputs_filtered ("Invalid Range!", stream);
>  	    break;

This is my opinion, so please feel free to disagree:

Using the rust-like syntax in the _INCLUSIVE cases ('=EXP') can be
a bit mysterious to someone not familiar with Rust. Or is it something
that's more widespread than I thought? If you agree, I'd like to
suggest we generate the range using the standard mathematical
notations instead, so it's language-agnostic and unequivocal.
We'd be changing it for all cases so that we always know whether
the bounds are inclusive or exclusive.

> diff --git a/gdb/expression.h b/gdb/expression.h
> index 7abd7f7503..86ee698aed 100644
> --- a/gdb/expression.h
> +++ b/gdb/expression.h
> @@ -150,15 +150,18 @@ extern void dump_prefix_expression (struct expression *, struct ui_file *);
>  
>  /* In an OP_RANGE expression, either bound could be empty, indicating
>     that its value is by default that of the corresponding bound of the
> -   array or string.  So we have four sorts of subrange.  This
> -   enumeration type is to identify this.  */
> -   
> +   array or string.  Also, the upper end of the range can be exclusive
> +   or inclusive.  So we have six sorts of subrange.  This enumeration
> +   type is to identify this.  */
> +
>  enum range_type
>    {
>      BOTH_BOUND_DEFAULT,		/* "(:)"  */
>      LOW_BOUND_DEFAULT,		/* "(:high)"  */
>      HIGH_BOUND_DEFAULT,		/* "(low:)"  */
> -    NONE_BOUND_DEFAULT		/* "(low:high)"  */
> +    NONE_BOUND_DEFAULT,		/* "(low:high)"  */
> +    NONE_BOUND_DEFAULT_INCLUSIVE, /* Rust "low..=high"  */
> +    LOW_BOUND_DEFAULT_INCLUSIVE, /* Rust "..=high"  */
>    };

Just a note to refer to my earlier email asking about the meaning
the previously existing enums (inclusive or exclusive), and perhaps
a suggestion to adjust the documentation above to make it unequivocal
by using the mathematical notation for each and everyone of them.


-- 
Joel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Add inclusive range support for Rust
  2018-04-25 16:27 ` Joel Brobecker
@ 2018-04-26 19:51   ` Tom Tromey
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2018-04-26 19:51 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> BOTH_BOUND_DEFAULT,		/* "(:)"  */
>> LOW_BOUND_DEFAULT,		/* "(:high)"  */
>> HIGH_BOUND_DEFAULT,		/* "(low:)"  */
>> -    NONE_BOUND_DEFAULT		/* "(low:high)"  */
>> +    NONE_BOUND_DEFAULT,		/* "(low:high)"  */
>> +    NONE_BOUND_DEFAULT_INCLUSIVE, /* Rust "low..=high"  */
>> +    LOW_BOUND_DEFAULT_INCLUSIVE, /* Rust "..=high"  */
>> };

Joel> Where the bounds exclusive before? The comments and the samples
Joel> of code I have been finding seem to indicate that the bounds
Joel> were already considered inclusive. But I can see how this is
Joel> not all that clear.

Yes, I think you are right -- they were inclusive for Fortran.
From value_f90_subarray:

  return value_slice (array, low_bound, high_bound - low_bound + 1);

What was weird then was that Rust treated them as exclusive, because at
the time Rust only had exclusive ranges.

I can change this and rename the new constants *_EXCLUSIVE.

Joel> Perhaps one way to clarify that is to use language-agnostic mathematical
Joel> notations for the ranges? Eg, using square brackets such as "[1:3[" or
Joel> perhaps "[1:3)" as I have sometimes seen?

I think it's simple to just write out some text explaining the meanings.
Then we won't need to worry whether someone knows the notation.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Add inclusive range support for Rust
  2018-04-25 16:52 ` Joel Brobecker
@ 2018-04-26 20:16   ` Tom Tromey
  2018-04-27 19:13     ` Joel Brobecker
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2018-04-26 20:16 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Using the rust-like syntax in the _INCLUSIVE cases ('=EXP') can be
Joel> a bit mysterious to someone not familiar with Rust. Or is it something
Joel> that's more widespread than I thought? If you agree, I'd like to
Joel> suggest we generate the range using the standard mathematical
Joel> notations instead, so it's language-agnostic and unequivocal.
Joel> We'd be changing it for all cases so that we always know whether
Joel> the bounds are inclusive or exclusive.

Instead of the Rust syntax or with notation (I find the notation a bit
easy to miss at times) I went with the wordier:

	  case LOW_BOUND_DEFAULT_EXCLUSIVE:
	    fputs_filtered ("ExclusiveRange '..EXP'", stream);
	    break;

And likewise for print_subexp_standard:

	if (range_type == NONE_BOUND_DEFAULT_EXCLUSIVE
	    || range_type == LOW_BOUND_DEFAULT_EXCLUSIVE)
	  fputs_filtered ("EXCLUSIVE_", stream);
	fputs_filtered ("RANGE(", stream);

I think it's fine to be wordy here because these dumpers are only gdb
debugging aids; users won't ordinarily see this output.

Joel> Just a note to refer to my earlier email asking about the meaning
Joel> the previously existing enums (inclusive or exclusive), and perhaps
Joel> a suggestion to adjust the documentation above to make it unequivocal
Joel> by using the mathematical notation for each and everyone of them.

I wrote comments like so:

    enum range_type
    {
      /* Neither the low nor the high bound was given -- so this refers to
         the entire available range.  */
      BOTH_BOUND_DEFAULT,
      /* The low bound was not given and the high bound is inclusive.  */
      LOW_BOUND_DEFAULT,
      /* The high bound was not given and the low bound in inclusive.  */
      HIGH_BOUND_DEFAULT,
      /* Both bounds were given and both are inclusive.  */
      NONE_BOUND_DEFAULT,
      /* The low bound was not given and the high bound is exclusive.  */
      NONE_BOUND_DEFAULT_EXCLUSIVE,
      /* Both bounds were given.  The low bound is inclusive and the high
         bound is exclusive.  */
      LOW_BOUND_DEFAULT_EXCLUSIVE,
    };

Here's the full patch, let me know what you think.

Tom

commit 30a0d1bf0ac2091cc63ba831b9015dae5e740fc1
Author: Tom Tromey <tom@tromey.com>
Date:   Thu Mar 29 14:14:07 2018 -0600

    Add inclusive range support for Rust
    
    This is version 2 of the patch to add inclusive range support for
    Rust.  I believe it addresses all review comments.
    
    Rust recently stabilized the inclusive range feature:
    
        https://github.com/rust-lang/rust/issues/28237
    
    An inclusive range is an expression like "..= EXPR" or "EXPR ..=
    EXPR".  It is like an ordinary range, except the upper bound is
    inclusive, not exclusive.
    
    This patch adds support for this feature to gdb.
    
    Regression tested on x86-64 Fedora 27.
    
    2018-04-26  Tom Tromey  <tom@tromey.com>
    
            PR rust/22545:
            * rust-lang.c (rust_inclusive_range_type_p): New function.
            (rust_range): Handle inclusive ranges.
            (rust_compute_range): Likewise.
            * rust-exp.y (struct rust_op) <inclusive>: New field.
            (DOTDOTEQ): New constant.
            (range_expr): Add "..=" productions.
            (operator_tokens): Add "..=" token.
            (ast_range): Add "inclusive" parameter.
            (convert_ast_to_expression) <case OP_RANGE>: Handle inclusive
            ranges.
            * parse.c (operator_length_standard) <case OP_RANGE>: Handle new
            bounds values.
            * expression.h (enum range_type) <NONE_BOUND_DEFAULT_EXCLUSIVE,
            LOW_BOUND_DEFAULT_EXCLUSIVE>: New constants.
            Update comments.
            * expprint.c (print_subexp_standard): Handle new bounds values.
            (dump_subexp_body_standard): Likewise.
    
    2018-04-26  Tom Tromey  <tom@tromey.com>
    
            PR rust/22545:
            * gdb.rust/simple.exp: Add inclusive range tests.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6164fc373a..4abdedca49 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,24 @@
+2018-04-26  Tom Tromey  <tom@tromey.com>
+
+	PR rust/22545:
+	* rust-lang.c (rust_inclusive_range_type_p): New function.
+	(rust_range): Handle inclusive ranges.
+	(rust_compute_range): Likewise.
+	* rust-exp.y (struct rust_op) <inclusive>: New field.
+	(DOTDOTEQ): New constant.
+	(range_expr): Add "..=" productions.
+	(operator_tokens): Add "..=" token.
+	(ast_range): Add "inclusive" parameter.
+	(convert_ast_to_expression) <case OP_RANGE>: Handle inclusive
+	ranges.
+	* parse.c (operator_length_standard) <case OP_RANGE>: Handle new
+	bounds values.
+	* expression.h (enum range_type) <NONE_BOUND_DEFAULT_EXCLUSIVE,
+	LOW_BOUND_DEFAULT_EXCLUSIVE>: New constants.
+	Update comments.
+	* expprint.c (print_subexp_standard): Handle new bounds values.
+	(dump_subexp_body_standard): Likewise.
+
 2018-04-26  Pedro Alves  <palves@redhat.com>
 
 	* elfread.c (elf_gnu_ifunc_resolver_return_stop): Use
diff --git a/gdb/expprint.c b/gdb/expprint.c
index c906904599..047ec11df1 100644
--- a/gdb/expprint.c
+++ b/gdb/expprint.c
@@ -578,9 +578,13 @@ print_subexp_standard (struct expression *exp, int *pos,
 	  longest_to_int (exp->elts[pc + 1].longconst);
 	*pos += 2;
 
+	if (range_type == NONE_BOUND_DEFAULT_EXCLUSIVE
+	    || range_type == LOW_BOUND_DEFAULT_EXCLUSIVE)
+	  fputs_filtered ("EXCLUSIVE_", stream);
 	fputs_filtered ("RANGE(", stream);
 	if (range_type == HIGH_BOUND_DEFAULT
-	    || range_type == NONE_BOUND_DEFAULT)
+	    || range_type == NONE_BOUND_DEFAULT
+	    || range_type == NONE_BOUND_DEFAULT_EXCLUSIVE)
 	  print_subexp (exp, pos, stream, PREC_ABOVE_COMMA);
 	fputs_filtered ("..", stream);
 	if (range_type == LOW_BOUND_DEFAULT
@@ -1099,12 +1103,18 @@ dump_subexp_body_standard (struct expression *exp,
 	  case LOW_BOUND_DEFAULT:
 	    fputs_filtered ("Range '..EXP'", stream);
 	    break;
+	  case LOW_BOUND_DEFAULT_EXCLUSIVE:
+	    fputs_filtered ("ExclusiveRange '..EXP'", stream);
+	    break;
 	  case HIGH_BOUND_DEFAULT:
 	    fputs_filtered ("Range 'EXP..'", stream);
 	    break;
 	  case NONE_BOUND_DEFAULT:
 	    fputs_filtered ("Range 'EXP..EXP'", stream);
 	    break;
+	  case NONE_BOUND_DEFAULT_EXCLUSIVE:
+	    fputs_filtered ("ExclusiveRange 'EXP..EXP'", stream);
+	    break;
 	  default:
 	    fputs_filtered ("Invalid Range!", stream);
 	    break;
diff --git a/gdb/expression.h b/gdb/expression.h
index 7abd7f7503..9f26bb8d60 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -150,15 +150,26 @@ extern void dump_prefix_expression (struct expression *, struct ui_file *);
 
 /* In an OP_RANGE expression, either bound could be empty, indicating
    that its value is by default that of the corresponding bound of the
-   array or string.  So we have four sorts of subrange.  This
-   enumeration type is to identify this.  */
-   
+   array or string.  Also, the upper end of the range can be exclusive
+   or inclusive.  So we have six sorts of subrange.  This enumeration
+   type is to identify this.  */
+
 enum range_type
-  {
-    BOTH_BOUND_DEFAULT,		/* "(:)"  */
-    LOW_BOUND_DEFAULT,		/* "(:high)"  */
-    HIGH_BOUND_DEFAULT,		/* "(low:)"  */
-    NONE_BOUND_DEFAULT		/* "(low:high)"  */
-  };
+{
+  /* Neither the low nor the high bound was given -- so this refers to
+     the entire available range.  */
+  BOTH_BOUND_DEFAULT,
+  /* The low bound was not given and the high bound is inclusive.  */
+  LOW_BOUND_DEFAULT,
+  /* The high bound was not given and the low bound in inclusive.  */
+  HIGH_BOUND_DEFAULT,
+  /* Both bounds were given and both are inclusive.  */
+  NONE_BOUND_DEFAULT,
+  /* The low bound was not given and the high bound is exclusive.  */
+  NONE_BOUND_DEFAULT_EXCLUSIVE,
+  /* Both bounds were given.  The low bound is inclusive and the high
+     bound is exclusive.  */
+  LOW_BOUND_DEFAULT_EXCLUSIVE,
+};
 
 #endif /* !defined (EXPRESSION_H) */
diff --git a/gdb/parse.c b/gdb/parse.c
index 1d53b5aa1a..193abe853f 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -995,6 +995,7 @@ operator_length_standard (const struct expression *expr, int endpos,
       switch (range_type)
 	{
 	case LOW_BOUND_DEFAULT:
+	case LOW_BOUND_DEFAULT_EXCLUSIVE:
 	case HIGH_BOUND_DEFAULT:
 	  args = 1;
 	  break;
@@ -1002,6 +1003,7 @@ operator_length_standard (const struct expression *expr, int endpos,
 	  args = 0;
 	  break;
 	case NONE_BOUND_DEFAULT:
+	case NONE_BOUND_DEFAULT_EXCLUSIVE:
 	  args = 2;
 	  break;
 	}
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index b661a803e3..56aa689a08 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -111,7 +111,8 @@ static const struct rust_op *ast_string (struct stoken str);
 static const struct rust_op *ast_struct (const struct rust_op *name,
 					 rust_set_vector *fields);
 static const struct rust_op *ast_range (const struct rust_op *lhs,
-					const struct rust_op *rhs);
+					const struct rust_op *rhs,
+					bool inclusive);
 static const struct rust_op *ast_array_type (const struct rust_op *lhs,
 					     struct typed_val_int val);
 static const struct rust_op *ast_slice_type (const struct rust_op *type);
@@ -300,6 +301,9 @@ struct rust_op
      name occurred at the end of the expression and is eligible for
      completion.  */
   unsigned int completing : 1;
+  /* For OP_RANGE, indicates whether the range is inclusive or
+     exclusive.  */
+  unsigned int inclusive : 1;
   /* Operands of expression.  Which one is used and how depends on the
      particular opcode.  */
   RUSTSTYPE left;
@@ -333,6 +337,7 @@ struct rust_op
 
 /* Operator tokens.  */
 %token <voidval> DOTDOT
+%token <voidval> DOTDOTEQ
 %token <voidval> OROR
 %token <voidval> ANDAND
 %token <voidval> EQEQ
@@ -382,7 +387,7 @@ struct rust_op
 %type <one_field_init> struct_expr_tail
 
 /* Precedence.  */
-%nonassoc DOTDOT
+%nonassoc DOTDOT DOTDOTEQ
 %right '=' COMPOUND_ASSIGN
 %left OROR
 %left ANDAND
@@ -535,13 +540,17 @@ array_expr:
 
 range_expr:
 	expr DOTDOT
-		{ $$ = ast_range ($1, NULL); }
+		{ $$ = ast_range ($1, NULL, false); }
 |	expr DOTDOT expr
-		{ $$ = ast_range ($1, $3); }
+		{ $$ = ast_range ($1, $3, false); }
+|	expr DOTDOTEQ expr
+		{ $$ = ast_range ($1, $3, true); }
 |	DOTDOT expr
-		{ $$ = ast_range (NULL, $2); }
+		{ $$ = ast_range (NULL, $2, false); }
+|	DOTDOTEQ expr
+		{ $$ = ast_range (NULL, $2, true); }
 |	DOTDOT
-		{ $$ = ast_range (NULL, NULL); }
+		{ $$ = ast_range (NULL, NULL, false); }
 ;
 
 literal:
@@ -956,6 +965,7 @@ static const struct token_info operator_tokens[] =
   { "&=", COMPOUND_ASSIGN, BINOP_BITWISE_AND },
   { "|=", COMPOUND_ASSIGN, BINOP_BITWISE_IOR },
   { "^=", COMPOUND_ASSIGN, BINOP_BITWISE_XOR },
+  { "..=", DOTDOTEQ, OP_NULL },
 
   { "::", COLONCOLON, OP_NULL },
   { "..", DOTDOT, OP_NULL },
@@ -1841,11 +1851,13 @@ ast_structop_anonymous (const struct rust_op *left,
 /* Make a range operation.  */
 
 static const struct rust_op *
-ast_range (const struct rust_op *lhs, const struct rust_op *rhs)
+ast_range (const struct rust_op *lhs, const struct rust_op *rhs,
+	   bool inclusive)
 {
   struct rust_op *result = OBSTACK_ZALLOC (work_obstack, struct rust_op);
 
   result->opcode = OP_RANGE;
+  result->inclusive = inclusive;
   result->left.op = lhs;
   result->right.op = rhs;
 
@@ -2473,13 +2485,22 @@ convert_ast_to_expression (struct parser_state *state,
 	  {
 	    convert_ast_to_expression (state, operation->right.op, top);
 	    if (kind == BOTH_BOUND_DEFAULT)
-	      kind = LOW_BOUND_DEFAULT;
+	      kind = (operation->inclusive
+		      ? LOW_BOUND_DEFAULT : LOW_BOUND_DEFAULT_EXCLUSIVE);
 	    else
 	      {
 		gdb_assert (kind == HIGH_BOUND_DEFAULT);
-		kind = NONE_BOUND_DEFAULT;
+		kind = (operation->inclusive
+			? NONE_BOUND_DEFAULT : NONE_BOUND_DEFAULT_EXCLUSIVE);
 	      }
 	  }
+	else
+	  {
+	    /* Nothing should make an inclusive range without an upper
+	       bound.  */
+	    gdb_assert (!operation->inclusive);
+	  }
+
 	write_exp_elt_opcode (state, OP_RANGE);
 	write_exp_elt_longcst (state, kind);
 	write_exp_elt_opcode (state, OP_RANGE);
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index cf8a15ee43..5d1c0a7f37 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -180,6 +180,17 @@ rust_range_type_p (struct type *type)
   return strcmp (TYPE_FIELD_NAME (type, i), "end") == 0;
 }
 
+/* Return true if TYPE is an inclusive range type, otherwise false.
+   This is only valid for types which are already known to be range
+   types.  */
+
+static bool
+rust_inclusive_range_type_p (struct type *type)
+{
+  return (strstr (TYPE_TAG_NAME (type), "::RangeInclusive") != NULL
+	  || strstr (TYPE_TAG_NAME (type), "::RangeToInclusive") != NULL);
+}
+
 /* Return true if TYPE seems to be the type "u8", otherwise false.  */
 
 static bool
@@ -1136,10 +1147,13 @@ rust_range (struct expression *exp, int *pos, enum noside noside)
   kind = (enum range_type) longest_to_int (exp->elts[*pos + 1].longconst);
   *pos += 3;
 
-  if (kind == HIGH_BOUND_DEFAULT || kind == NONE_BOUND_DEFAULT)
+  if (kind == HIGH_BOUND_DEFAULT || kind == NONE_BOUND_DEFAULT
+      || kind == NONE_BOUND_DEFAULT_EXCLUSIVE)
     low = evaluate_subexp (NULL_TYPE, exp, pos, noside);
-  if (kind == LOW_BOUND_DEFAULT || kind == NONE_BOUND_DEFAULT)
+  if (kind == LOW_BOUND_DEFAULT || kind == LOW_BOUND_DEFAULT_EXCLUSIVE
+      || kind == NONE_BOUND_DEFAULT || kind == NONE_BOUND_DEFAULT_EXCLUSIVE)
     high = evaluate_subexp (NULL_TYPE, exp, pos, noside);
+  bool inclusive = (kind == NONE_BOUND_DEFAULT || kind == LOW_BOUND_DEFAULT);
 
   if (noside == EVAL_SKIP)
     return value_from_longest (builtin_type (exp->gdbarch)->builtin_int, 1);
@@ -1154,7 +1168,8 @@ rust_range (struct expression *exp, int *pos, enum noside noside)
       else
 	{
 	  index_type = value_type (high);
-	  name = "std::ops::RangeTo";
+	  name = (inclusive
+		  ? "std::ops::RangeToInclusive" : "std::ops::RangeTo");
 	}
     }
   else
@@ -1169,7 +1184,7 @@ rust_range (struct expression *exp, int *pos, enum noside noside)
 	  if (!types_equal (value_type (low), value_type (high)))
 	    error (_("Range expression with different types"));
 	  index_type = value_type (low);
-	  name = "std::ops::Range";
+	  name = inclusive ? "std::ops::RangeInclusive" : "std::ops::Range";
 	}
     }
 
@@ -1245,6 +1260,9 @@ rust_compute_range (struct type *type, struct value *range,
       *kind = (*kind == BOTH_BOUND_DEFAULT
 	       ? LOW_BOUND_DEFAULT : NONE_BOUND_DEFAULT);
       *high = value_as_long (value_field (range, i));
+
+      if (rust_inclusive_range_type_p (type))
+	++*high;
     }
 }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 34da102c62..43fcf3b940 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-04-26  Tom Tromey  <tom@tromey.com>
+
+	PR rust/22545:
+	* gdb.rust/simple.exp: Add inclusive range tests.
+
 2018-04-26  Pedro Alves  <palves@redhat.com>
 
 	* gdb.base/gnu-ifunc.exp (set-break): Test that GDB resolves
diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
index d70de33835..ba90e061ce 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -219,7 +219,9 @@ gdb_test "print r###\"###hello\"##" "Unexpected EOF in string"
 gdb_test "print r###\"hello###" "Unexpected EOF in string"
 
 gdb_test "print 0..5" " = .*::ops::Range.* \\{start: 0, end: 5\\}"
+gdb_test "print 0..=5" " = .*::ops::RangeInclusive.* \\{start: 0, end: 5\\}"
 gdb_test "print ..5" " = .*::ops::RangeTo.* \\{end: 5\\}"
+gdb_test "print ..=5" " = .*::ops::RangeToInclusive.* \\{end: 5\\}"
 gdb_test "print 5.." " = .*::ops::RangeFrom.* \\{start: 5\\}"
 gdb_test "print .." " = .*::ops::RangeFull"
 
@@ -244,7 +246,9 @@ proc test_one_slice {svar length base range} {
 }
 
 test_one_slice slice 1 w 2..3
+test_one_slice slice 1 w 2..=2
 test_one_slice slice2 1 slice 0..1
+test_one_slice slice2 1 slice 0..=0
 
 test_one_slice all1 4 w ..
 test_one_slice all2 1 slice ..
@@ -253,7 +257,9 @@ test_one_slice from1 3 w 1..
 test_one_slice from2 0 slice 1..
 
 test_one_slice to1 3 w ..3
+test_one_slice to1 3 w ..=2
 test_one_slice to2 1 slice ..1
+test_one_slice to2 1 slice ..=0
 
 gdb_test "print w\[2..3\]" "Can't take slice of array without '&'"
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Add inclusive range support for Rust
  2018-04-26 20:16   ` Tom Tromey
@ 2018-04-27 19:13     ` Joel Brobecker
  0 siblings, 0 replies; 9+ messages in thread
From: Joel Brobecker @ 2018-04-27 19:13 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> Instead of the Rust syntax or with notation (I find the notation a bit
> easy to miss at times) I went with the wordier:
> 
> 	  case LOW_BOUND_DEFAULT_EXCLUSIVE:
> 	    fputs_filtered ("ExclusiveRange '..EXP'", stream);
> 	    break;
> 
> And likewise for print_subexp_standard:
> 
> 	if (range_type == NONE_BOUND_DEFAULT_EXCLUSIVE
> 	    || range_type == LOW_BOUND_DEFAULT_EXCLUSIVE)
> 	  fputs_filtered ("EXCLUSIVE_", stream);
> 	fputs_filtered ("RANGE(", stream);
> 
> I think it's fine to be wordy here because these dumpers are only gdb
> debugging aids; users won't ordinarily see this output.

That looks good to me.

> Joel> Just a note to refer to my earlier email asking about the meaning
> Joel> the previously existing enums (inclusive or exclusive), and perhaps
> Joel> a suggestion to adjust the documentation above to make it unequivocal
> Joel> by using the mathematical notation for each and everyone of them.
> 
> I wrote comments like so:
> 
>     enum range_type
>     {
>       /* Neither the low nor the high bound was given -- so this refers to
>          the entire available range.  */
>       BOTH_BOUND_DEFAULT,
>       /* The low bound was not given and the high bound is inclusive.  */
>       LOW_BOUND_DEFAULT,
>       /* The high bound was not given and the low bound in inclusive.  */
>       HIGH_BOUND_DEFAULT,
>       /* Both bounds were given and both are inclusive.  */
>       NONE_BOUND_DEFAULT,
>       /* The low bound was not given and the high bound is exclusive.  */
>       NONE_BOUND_DEFAULT_EXCLUSIVE,
>       /* Both bounds were given.  The low bound is inclusive and the high
>          bound is exclusive.  */
>       LOW_BOUND_DEFAULT_EXCLUSIVE,
>     };

Good idea! I think it's much clearer.

>     2018-04-26  Tom Tromey  <tom@tromey.com>
>     
>             PR rust/22545:
>             * rust-lang.c (rust_inclusive_range_type_p): New function.
>             (rust_range): Handle inclusive ranges.
>             (rust_compute_range): Likewise.
>             * rust-exp.y (struct rust_op) <inclusive>: New field.
>             (DOTDOTEQ): New constant.
>             (range_expr): Add "..=" productions.
>             (operator_tokens): Add "..=" token.
>             (ast_range): Add "inclusive" parameter.
>             (convert_ast_to_expression) <case OP_RANGE>: Handle inclusive
>             ranges.
>             * parse.c (operator_length_standard) <case OP_RANGE>: Handle new
>             bounds values.
>             * expression.h (enum range_type) <NONE_BOUND_DEFAULT_EXCLUSIVE,
>             LOW_BOUND_DEFAULT_EXCLUSIVE>: New constants.
>             Update comments.
>             * expprint.c (print_subexp_standard): Handle new bounds values.
>             (dump_subexp_body_standard): Likewise.
>     
>     2018-04-26  Tom Tromey  <tom@tromey.com>
>     
>             PR rust/22545:
>             * gdb.rust/simple.exp: Add inclusive range tests.

The patch looks good to me, so you can go ahead and push.

Thanks Tom!

-- 
Joel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-04-27 19:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-29 20:16 [RFA] Add inclusive range support for Rust Tom Tromey
2018-04-17 19:48 ` Tom Tromey
2018-04-25 15:33   ` Tom Tromey
2018-04-25 16:04     ` Joel Brobecker
2018-04-25 16:27 ` Joel Brobecker
2018-04-26 19:51   ` Tom Tromey
2018-04-25 16:52 ` Joel Brobecker
2018-04-26 20:16   ` Tom Tromey
2018-04-27 19:13     ` Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).