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: Joel Brobecker <brobecker at adacore dot com>
- To: Tom Tromey <tom at tromey dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 27 Apr 2018 12:13:02 -0700
- Subject: Re: [RFA] Add inclusive range support for Rust
- References: <20180329201609.13699-1-tom@tromey.com> <20180425165205.cwkvthuotlpaq67z@adacore.com> <87d0yl7mfp.fsf@tromey.com>
> 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