This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Add inclusive range support for Rust
- From: Tom Tromey <tom at tromey dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: Tom Tromey <tom at tromey dot com>, gdb-patches at sourceware dot org
- Date: Thu, 26 Apr 2018 14:16:42 -0600
- Subject: Re: [RFA] Add inclusive range support for Rust
- References: <20180329201609.13699-1-tom@tromey.com> <20180425165205.cwkvthuotlpaq67z@adacore.com>
>>>>> "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 '&'"