This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Add inclusive range support for Rust


> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]