public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Tom Tromey <tom@tromey.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA] Add inclusive range support for Rust
Date: Fri, 27 Apr 2018 19:13:00 -0000	[thread overview]
Message-ID: <20180427191302.niioyc6dunzhai6o@adacore.com> (raw)
In-Reply-To: <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

      reply	other threads:[~2018-04-27 19:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-29 20:16 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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180427191302.niioyc6dunzhai6o@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).