From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 87922 invoked by alias); 26 Apr 2018 20:16:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 87470 invoked by uid 89); 26 Apr 2018 20:16:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=mysterious, Cant, stabilized, subrange X-HELO: gateway36.websitewelcome.com Received: from gateway36.websitewelcome.com (HELO gateway36.websitewelcome.com) (50.116.127.2) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 26 Apr 2018 20:16:44 +0000 Received: from cm10.websitewelcome.com (cm10.websitewelcome.com [100.42.49.4]) by gateway36.websitewelcome.com (Postfix) with ESMTP id 8976E41F3A58A for ; Thu, 26 Apr 2018 15:16:43 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id BnJrfcTI56il3BnJrfc8tv; Thu, 26 Apr 2018 15:16:43 -0500 X-Authority-Reason: nr=8 Received: from 97-122-176-117.hlrn.qwest.net ([97.122.176.117]:58908 helo=pokyo) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89_1) (envelope-from ) id 1fBnJr-002Ch7-7r; Thu, 26 Apr 2018 15:16:43 -0500 From: Tom Tromey To: Joel Brobecker Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [RFA] Add inclusive range support for Rust References: <20180329201609.13699-1-tom@tromey.com> <20180425165205.cwkvthuotlpaq67z@adacore.com> Date: Thu, 26 Apr 2018 20:16:00 -0000 In-Reply-To: <20180425165205.cwkvthuotlpaq67z@adacore.com> (Joel Brobecker's message of "Wed, 25 Apr 2018 09:52:05 -0700") Message-ID: <87d0yl7mfp.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-BWhitelist: no X-Source-L: No X-Exim-ID: 1fBnJr-002Ch7-7r X-Source-Sender: 97-122-176-117.hlrn.qwest.net (pokyo) [97.122.176.117]:58908 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-SW-Source: 2018-04/txt/msg00540.txt.bz2 >>>>> "Joel" == Joel Brobecker 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 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 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) : New field. (DOTDOTEQ): New constant. (range_expr): Add "..=" productions. (operator_tokens): Add "..=" token. (ast_range): Add "inclusive" parameter. (convert_ast_to_expression) : Handle inclusive ranges. * parse.c (operator_length_standard) : Handle new bounds values. * expression.h (enum range_type) : New constants. Update comments. * expprint.c (print_subexp_standard): Handle new bounds values. (dump_subexp_body_standard): Likewise. 2018-04-26 Tom Tromey 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 + + 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) : New field. + (DOTDOTEQ): New constant. + (range_expr): Add "..=" productions. + (operator_tokens): Add "..=" token. + (ast_range): Add "inclusive" parameter. + (convert_ast_to_expression) : Handle inclusive + ranges. + * parse.c (operator_length_standard) : Handle new + bounds values. + * expression.h (enum range_type) : New constants. + Update comments. + * expprint.c (print_subexp_standard): Handle new bounds values. + (dump_subexp_body_standard): Likewise. + 2018-04-26 Pedro Alves * 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 DOTDOT +%token DOTDOTEQ %token OROR %token ANDAND %token EQEQ @@ -382,7 +387,7 @@ struct rust_op %type 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 + + PR rust/22545: + * gdb.rust/simple.exp: Add inclusive range tests. + 2018-04-26 Pedro Alves * 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 '&'"