cc @mark-i-m

nikomatsakis on 8 Feb 2018

@nikomatsakis Could you update the checklist? The preliminary impl is available on nightly.

mark-i-m on 17 Feb 2018

👍1

@Centril I think you need to update the tracking issue in the RFC, too :stuck_out_tongue:

mark-i-m on 27 Feb 2018

@mark-i-m Already done 🤣

Centril on 27 Feb 2018

👍1

Is there any plan to stabilise this soon?

clarfonthey on 28 Mar 2018

I think it depends on experience if this is the right way to solve the problems involved... Frankly, I haven't really written any macros since I implemented this :stuck_out_tongue: ...

I think there is still the one problem that was pointed out in the RFC thread: this doesn't provide a clean solution for trailing commas in * macros, since you would end up accepting foo!{,} which is a bit weird.

We could conceivably stabilize this anyway if people find it really useful. Any thoughts?

mark-i-m on 28 Mar 2018

I personally see it more ergonomic to have rules for () and $(...),+ $(,)? than $(...),* and $(...,)*, as the former usually requires way less duplication than the latter. The former it also natural for recursive macros, and it basically solves the trailing comma problem nicely.

Considering how it also allows optional arguments as well, I think it's pretty much a win here.

clarfonthey on 28 Mar 2018

👍1

As far as the one unresolved question (following separators) goes, I'm in favour of allowing it for the sake of a simpler grammar, but linting it in rustc (not clippy). That seems pretty ergonomic to me, and the compiler internally would have to accept it with a separator anyways to provide useful error messages.

clarfonthey on 28 Mar 2018

👍1

I don’t see the point of allowing the trailing separator even with a lint. It’s totally meaningless as you point out, so its existence in code would probably just perplex readers!

alexreg on 5 Apr 2018

👍1

the compiler internally would have to accept it with a separator anyways to provide useful error messages.

Hmm... I guess this is a good point. It means that probably the code complexity (in the compiler) of keeping or not keeping the separator is about the same. Still, having a simpler grammar is nice, especially if libsyntax2.0 ever actually intends have something auto-generated.

What about a deny-by-default lint?

mark-i-m on 5 Apr 2018

When would anyone ever allow the lint, though? This kinda sounds like "we can't decide, just make it configurable".

durka on 5 Apr 2018

👍1

When would anyone ever allow the lint, though?

My guess: probably never. On the other hand, when is anyone likely to actually use a separator for a single item? Will anyone ever run into the lint? The tradeoff is between a mildly simpler grammar and mildly simpler UX.

mark-i-m on 5 Apr 2018

The changes if we want disallow it are pretty minimal:

diff --git a/src/libsyntax/ext/tt/quoted.rs b/src/libsyntax/ext/tt/quoted.rs
index f324ede..a6fb782 100644
--- a/src/libsyntax/ext/tt/quoted.rs
+++ b/src/libsyntax/ext/tt/quoted.rs
@@ -470,8 +470,11 @@ where
                         GateIssue::Language,
                         explain,
                     );
+                } else {
+                    sess.span_diagnostic
+                        .span_err(span, "`?` macro repetition does not allow a separator");
                 }
-                return (Some(tok), op);
+                return (None, op);
             }
             Ok(Ok(op)) => return (Some(tok), op),

mark-i-m on 5 Apr 2018

My guess: probably never. On the other hand, when is anyone likely to actually use a separator for a single item? Will anyone ever run into the lint? The tradeoff is between a mildly simpler grammar and mildly simpler UX.

I think this is really "designer's/developer's motivation" vs. "user's motivation". And making the user happy should always win, I think. After all, there's a fair bit of ugly stuff in most compilers (including the Rust one), but most people don't care about it, because they don't touch it, and if they do they expect a significant learning curve. A compiler's end goal is to support a language rather than have an elegant internal structure (which is more of a bonus). The fact is, if you want to go messing with the compiler or libsyntax, taking the time to learn the slight difference between ? and */+ is no big deal. If you're a user, especially one new to Rust, that could be a real gotcha.

Now, since from the Rust user's perspective this feature adds literally nothing, and adding an error (as @mark-i-m's diff shows) is very straightforward, I maintain we disallow it. @durka's point and the article he links to is also a good one. Configuration is only a good thing when it's absolutely needed. Convention over it any other time.

alexreg on 5 Apr 2018

I went back and re-read part of the RFC thread, and this comment stuck out to me: https://github.com/rust-lang/rfcs/pull/2298#issuecomment-363965784

There is the question of what this matches: $(a)?+. It could be:

a+ (hypothetical)
a?a?a (current implementation)

Allowing the separator actually allows us to disambiguate:

$(a),?+ matches a+
$(a)?+ matches a?a?a

But at the same time, this seems really dubious, since removing the separator seems like it should not change the pattern matched. If we disallow separator on ?, then things are a bit cleaner:

$(a),?+ is parsing error
$(a)?+ matches a?a?a

Admittedly, this is all a bit contrived, but it does seem to throw the balance in favor of disallowing a separator on ?...

EDIT: corrected typo

mark-i-m on 5 Apr 2018

Agreed that if removing the separator changes things that seems weird and bad. But it's a shame that in the latter scenario you can't match aa+...

durka on 5 Apr 2018

👍1

One option would be to change the disambiguation strategy. That is, we stop allowing ? as a separator for any of the Kleene operators. This would be a breaking change, but perhaps it might not be that bad?

mark-i-m on 5 Apr 2018

We could allow precisely one separator, namely ?, for this edge case:
$(a)?? + matches a+ or aa+. That eliminates the trouble where an
unrelated separator has effects. Getting a little hacky though.

On Thu, Apr 5, 2018 at 4:20 PM, Who? Me?! notifications@github.com wrote:

One option would be to change the disambiguation strategy. That is, we
stop allowing ? as a separator for any of the Kleene operators. This
would be a breaking change, but perhaps it might not be that bad?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/rust/issues/48075#issuecomment-379064006,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC3n2Z5dotnKZCdGgLKn_SW42RzRPboks5tlnyqgaJpZM4R-yf0
.

durka on 5 Apr 2018

Now that you mention that, maybe a two-character ?? operator makes the most sense. No ambiguity, no allowance for specification of meaningless separators, and no prevention of using ? as a separator for the other Kleene operators.

alexreg on 5 Apr 2018

We could allow precisely one separator, namely ?, for this edge case: $(a)?? + matches a+ or aa+. That eliminates the trouble where an unrelated separator has effects. Getting a little hacky though.

Ooof. I would like to not have this kind of corner case.

Now that you mention that, maybe a two-character ?? operator makes the most sense.

But one of the reasons we chose ? to begin with is that it is a well-known regex operator. I feel like ?? gives up at least some of that familiarity.

Overall, both of these suggests _add_ complexity to the grammar _and_ break conformity with the other Kleene operators. IMHO, simply disallowing separators with ? is a cleaner solution.

mark-i-m on 6 Apr 2018

Overall, both of these suggests add complexity to the grammar and break conformity with the other Kleene operators. IMHO, simply disallowing separators with ? is a cleaner solution.

If you think this is okay, then sure. Another alternative would be a two-character operator _? or something, but up to you...

alexreg on 6 Apr 2018

Would the right approach be for me to make a PR with the necessary changes and get a crater run done?

mark-i-m on 6 Apr 2018

@mark-i-m I'm probably not the one to best provide an answer, but that sounds like a sensible route to me! If that crater run passes, we can get this merged & stabilised right away. I presume it's already enabled for macro_rules! and the new Macros 2.0 macro statements?

alexreg on 6 Apr 2018

Ok, let me try that... so in summary we are proposing the following changes:

Disallow ? as a separator
Disallow ? from taking a separator

mark-i-m on 6 Apr 2018

👍1

Wait, why is it necessary to disallow ? as a separator? Probably rare
but...

On Thu, Apr 5, 2018, 19:36 Who? Me?! notifications@github.com wrote:

Ok, let me try that... so in summary we are proposing the following
changes:

Disallow ? as a separator

Disallow ? from taking a separator

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/rust/issues/48075#issuecomment-379107325,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC3n9LntalgXBbWHrUX-xYlHOvN-piGks5tlqpxgaJpZM4R-yf0
.

durka on 6 Apr 2018

It allows you to do $(a)?+ to match a+ rather than a?a?a... I guess this is kind of a guess at which is most useful. Do you have a preference?

mark-i-m on 6 Apr 2018

Wait, why is it necessary to disallow ? as a separator? Probably rare
but...

Parsing ambiguity when ? is intended as the question mark operator but followed by a * or +.

alexreg on 6 Apr 2018

Ok, let me try that... so in summary we are proposing the following changes:

Disallow ? as a separator

Disallow ? from taking a separator

Yep, sounds good to me.

alexreg on 6 Apr 2018

Parsing ambiguity when ? is intended as the question mark operator but followed by a * or +.

Right. The current disambiguation is that $(a)?X is a repetition with ? as the separator if X is a Kleene operator, and it is a ? repetition with no separator if X is not a Kleene operator.

mark-i-m on 6 Apr 2018

Disallow ? as a separator

Disallow ? from taking a separator

Made PR #49719 as a test implementation of this.

mark-i-m on 6 Apr 2018

Right. The current disambiguation is that $(a)?X is a repetition with ? as the separator if X is a Kleene operator, and it is a ? repetition with no separator if X is not a Kleene operator.

An unfortunate use of the ? character as a placeholder, given this PR, perhaps, but yep! 😉

Anyway, this of course implies that * and + can't be used as separators currently. So it seems quite consistent to move forwards by not allowing ? as a separator either, right? That is, not a "moral" design problem at least.

alexreg on 6 Apr 2018

No, but it is breaking change, which is a "moral" design problem :stuck_out_tongue:

mark-i-m on 6 Apr 2018

I think we're interpreting "moral" in a different way. That's not "moral" to me (in the sense of being consistent to one's principles), but rather a pragmatic problem.

alexreg on 6 Apr 2018

Ok, so the crater run in #49719 shows that disallowing ? as a separator doesn't seem to cause any regressions. This is not too surprising.

So what is the next step? @nikomatsakis ?

mark-i-m on 15 Apr 2018

@mark-i-m, In case Niko doesn't get around to this in the next few days, I'd suggest you're in a good position to request merge approval by a team member now (the review is really very straightforward).

alexreg on 15 Apr 2018

@alexreg Niko will be on vacation next week. =)

Centril on 15 Apr 2018

@Centril Do you know what the next steps are?

mark-i-m on 15 Apr 2018

@mark-i-m The next steps are to wait for someone who is not me ;)

Centril on 15 Apr 2018

😄2

Ok, it looks like the PR merged somehow without an FCP... I think we can play around with this and see if it is intuitive to use.

mark-i-m on 16 Apr 2018

I love how there's a *** Do not merge *** message in the master commit log now. 😄

But anyway, I think we're pretty safe, given what the crater run returned...

alexreg on 16 Apr 2018

😄3 👍2

Yep, I agree. I think the risk is pretty low, and we have a couple of months to fix it if there turns out to be more regressions than needed.

mark-i-m on 16 Apr 2018

👍1

Also posting on this, https://github.com/rust-lang/rust/pull/49719 is going to be reverted (rationale in https://github.com/rust-lang/rust/issues/51416#issue-330308198).

pietroalbini on 7 Jun 2018

@pietroalbini @nikomatsakis Was there any discussion on the team of what the preferred path forward is?

My proposal is that prohibit ? as a separator for the 2018 edition (or on nightly 2015 with a feature flag). We can then make ? always be a Kleene operator.

@alexreg

Funny aside:

I love how there's a * Do not merge * message in the master commit log now. :smile:

$ git log | grep -i 'do not merge'
    **Do not merge** (yet)
    Implementing rust-lang/rfcs#2349 (do not merge until RFC is merged)
    **DO NOT MERGE YET**
    ~~✨ work in progress, please do not merge ✨~~
    **NOTE**: DO NOT MERGE before we get the new beta as the stage0, there's some cruft to remove.
    Do not merge yet.
    Do not merge before the referenced PR is merged. I will fix the PR once that is merged (or close if it is not)
    First part of the improvement. I then intend to improve resolve_error as indicated by @eddyb. Do not merge for now (please !).
    Note: Do not merge until we get a newer snapshot that includes #21374

mark-i-m on 8 Jun 2018

😄3

@mark-i-m chat log when this was discussed. Quoting @nikomatsakis at the end:

in short, I think we should revert first, and then reapproach the question of Rust 2018
we would need to have a migration lint
it becomes part of the edition planning
basically I think we should not conflate these two things
the PR landed in error without an official decision; we should fix that,
then we can make the decision.

pietroalbini on 8 Jun 2018

Funny aside

I think I ran that myself out of curiosity, after I saw this merged, haha. Maybe editing those commit messages would be in order now, not to freak people out too much. 😉

alexreg on 8 Jun 2018

😄1

Off-topic

That would be changing git history, which is bad.

mark-i-m on 9 Jun 2018

Thanks @pietroalbini.

So what would a migration lint look like? Would we just suggest using any of the other valid separators in place of ?? I can write that up, but I'm not sure exactly what that would look like...

mark-i-m on 9 Jun 2018

That would be changing git history, which is bad.

Yeah, it wasn't a serious suggestion. I would do it on a small project, but not one of this size!

P.S. How do you do that "off-topic" dropdown thing?

alexreg on 9 Jun 2018

```html

Off-topic

That would be changing git history, which is bad.

mark-i-m on 9 Jun 2018

Off-topic

Thanks! Sucks that one has to use HTML to do this though. 😛

alexreg on 9 Jun 2018

😄1

First attempt to impl: #51587

mark-i-m on 16 Jun 2018

I came across an interesting use case for this: https://github.com/rust-lang/rust/blob/01cc982e936120acb0424e41de14e42ba2d88c6f/src/libsyntax/ext/expand.rs#L43-L51

I don't know of any way to express this macro at all without ? repetition...

mark-i-m on 25 Jun 2018

Hmm... would you need to take the ? parts as :tt and pass them to another macro or something?

mark-i-m on 25 Jun 2018

It's not that hard, just a lot more verbose. You split at ; and pass segments to another macro that can match on one/many/fn.

durka on 25 Jun 2018

👍1

Request for comments/FCP on newest implementation: #51934

mark-i-m on 30 Jun 2018

Edition dependent behavior has merged. Please play with it.

mark-i-m on 26 Jul 2018

👍1

I just tried to compile recent rust-1.28.0 from sources, and got an error related to this issue it seems:

   Compiling toml v0.4.6
   Compiling idna v0.1.4
   Compiling rustc_version v0.2.2
   Compiling backtrace v0.3.6
   Compiling url v1.7.0
   Compiling error-chain v0.11.0
   Compiling clippy v0.0.202 (file:///var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/src/tools/clippy)
   Compiling cargo_metadata v0.5.4
   Compiling clippy_lints v0.0.202 (file:///var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/src/tools/clippy/clippy_lints)
error[E0658]: Using the `?` macro Kleene operator for "at most one" repetition is unstable (see issue #48075)
 --> <declare_lint macros>:6:37
  |
6 | : expr => $ edition_level : ident $ ( , ) ? ) => (
  |                                     ^^^^^
  |
  = help: add #![feature(macro_at_most_once_rep)] to the crate attributes to enable

error[E0658]: Using the `?` macro Kleene operator for "at most one" repetition is unstable (see issue #48075)
 --> <lint_array macros>:1:29
  |
1 | ( $ ( $ lint : expr ) , * $ ( , ) ? ) => {
  |                             ^^^^^
  |
  = help: add #![feature(macro_at_most_once_rep)] to the crate attributes to enable

error[E0658]: Using the `?` macro Kleene operator for "at most one" repetition is unstable (see issue #48075)
 --> <declare_lint macros>:6:37
  |
6 | : expr => $ edition_level : ident $ ( , ) ? ) => (
  |                                     ^^^^^
  |
  = help: add #![feature(macro_at_most_once_rep)] to the crate attributes to enable

error[E0658]: Using the `?` macro Kleene operator for "at most one" repetition is unstable (see issue #48075)
 --> <lint_array macros>:1:29
  |
1 | ( $ ( $ lint : expr ) , * $ ( , ) ? ) => {
  |                             ^^^^^
  |
  = help: add #![feature(macro_at_most_once_rep)] to the crate attributes to enable

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0658`.
error: Could not compile `clippy_lints`.

Caused by:
  process didn't exit successfully: `/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/bootstrap/debug/rustc --crate-name clippy_lints tools/clippy/clippy_lints/src/lib.rs --error-format json --crate-type lib --emit=dep-info,link -C opt-level=2 -C metadata=808dbce065b60ad1 -C extra-filename=-808dbce065b60ad1 --out-dir /var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps -L dependency=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps --extern pulldown_cmark=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libpulldown_cmark-ebba01dbabb50f38.rlib --extern regex_syntax=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libregex_syntax-7768c532a4d73cb8.rlib --extern if_chain=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libif_chain-08067b1ac6a3dc39.rlib --extern serde=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libserde-954bbc780b0020e8.rlib --extern url=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/liburl-c708e1a067dd3842.rlib --extern cargo_metadata=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libcargo_metadata-b439e4a4bb7ed895.rlib --extern matches=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libmatches-fbf9546c1a654d6c.rlib --extern itertools=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libitertools-5fabdd39b0d58296.rlib --extern toml=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libtoml-62bd5734009e1cd8.rlib --extern unicode_normalization=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libunicode_normalization-912710911145d109.rlib --extern lazy_static=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/liblazy_static-8fc7d2db85a0ad9d.rlib --extern quine_mc_cluskey=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libquine_mc_cluskey-e46248baeb23cead.rlib --extern serde_derive=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libserde_derive-fbad2b99bfa6d574.so --extern semver=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libsemver-6d8f137054c75b61.rlib -L native=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/build/backtrace-sys-e9110a0f73074a74/out` (exit code: 101)
warning: build failed, waiting for other jobs to finish...
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0658`.
error: Could not compile `clippy_lints`.

Caused by:
  process didn't exit successfully: `/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/bootstrap/debug/rustc --crate-name clippy_lints tools/clippy/clippy_lints/src/lib.rs --error-format json --crate-type lib --emit=dep-info,link -C opt-level=2 -C metadata=5926decf1915fdac -C extra-filename=-5926decf1915fdac --out-dir /var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps -L dependency=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps --extern pulldown_cmark=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libpulldown_cmark-d415139627f93b01.rlib --extern regex_syntax=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libregex_syntax-9b09cd5290257a50.rlib --extern if_chain=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libif_chain-4e5915875d9f2d56.rlib --extern serde=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libserde-4cac8b64472a54f4.rlib --extern url=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/liburl-b24996703ea8897a.rlib --extern cargo_metadata=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libcargo_metadata-616b5536710c1ae0.rlib --extern matches=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libmatches-51d09f94fb6f26c4.rlib --extern itertools=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libitertools-cb13292f884d6995.rlib --extern toml=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libtoml-9c6e4743a4b1d972.rlib --extern unicode_normalization=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libunicode_normalization-5dac60218d700ffd.rlib --extern lazy_static=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/liblazy_static-59933e6c610d3f59.rlib --extern quine_mc_cluskey=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libquine_mc_cluskey-16cd1338e556efc4.rlib --extern serde_derive=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libserde_derive-fbad2b99bfa6d574.so --extern semver=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libsemver-2af0d4edd8c5a4c3.rlib -L native=/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/build/x86_64-unknown-linux-gnu/stage2-tools/release/build/backtrace-sys-1bf1db127596f6c1/out` (exit code: 101)
command did not execute successfully: "/var/tmp/portage/dev-lang/rust-1.28.0/work/rust-stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--frozen" "--manifest-path" "/var/tmp/portage/dev-lang/rust-1.28.0/work/rustc-1.28.0-src/src/tools/clippy/Cargo.toml" "--features" "" "--message-format" "json"
expected success, got: exit code: 101
Set({"src/tools/rustfmt"}) not skipped for "tool::Rustfmt" -- not in ["src/tools/miri"]
Building stage2 tool rustfmt (x86_64-unknown-linux-gnu)

oddly enough, the build finished without further breakage. full build.log: build.log.zip

stefson on 6 Aug 2018

I just tried to compile recent rust-1.28.0 from sources, and got an error related to this issue it seems:

Are you bootstrapping correctly? Because the CI should fail if this were happening elsewhere...

alexreg on 6 Aug 2018

I'm not really sure what you mean by that, but does the build.log help you to understand what is happening? I'm a gentoo user (not a dev), and this is what the rust ebuild gave me.

stefson on 6 Aug 2018

Okay, so you were just trying to install the Rust package on Gentoo? This may be a problem with the package specification/scripts then.

alexreg on 6 Aug 2018

It is a full compile of rustc and rust-std plus cargo in this case, with the help of a prebuilt stage0 I believe. The devs of the package aren't so sure either what to do with this, I may ping them if you want to speak to them directly?

stefson on 6 Aug 2018

@stefson It may be an issue to due with stage0 being prebuilt. It's probably better to speak to a senior team member like @nikomatsakis or @alexcrichton, who might be able to help you and the devs (no promises).

alexreg on 6 Aug 2018

@stefson the 1.28.0 compiler bootstraps from 1.27.2, you'll want to make sure you're using the right version.

alexcrichton on 7 Aug 2018

rustdoc doesn't consider ? a macro repetition operator, thus chokes on any macro including it. (This applies to private macros as well.)

CAD97 on 20 Aug 2018

@CAD97 Would you kindly submit a PR for that over at the rustdoc repo?

alexreg on 21 Aug 2018

Is rustdoc not part of the rust-lang/rust repo? I thought it was (or at least there are T-rustdoc issues in this repo and I didn't find a rustdoc repo elsewhere other than the rustdoc2/doxidize one). I could open an issue for that specifically if that's what you're asking?

EDIT: This was fixed sometime between the nightly I was using and now, it's no longer an issue.

CAD97 on 21 Aug 2018

It is a part of this repo. But frankly, I'm confused as to why it doesn't work. rustdoc uses the compiler.

What errors are you seeing exactly?

mark-i-m on 21 Aug 2018

Using the exact nightly I was, a cargo check passed but a cargo doc complained that "? is not a macro repetition operator". This no longer happens on the current nightly, so is no longer an issue.

CAD97 on 21 Aug 2018

Hmm how interesting. It sounds like rustdoc was not getting the correct edition/feature gates or something. That error is emitted only on 2015 or without the feature gate.

Glad it's working now.

mark-i-m on 21 Aug 2018

@CAD97 Yeah, it was pretty late when I wrote that; please ignore me!

alexreg on 21 Aug 2018

I have to say I'm not super happy with the decision to do edition breakage here...
We found zero regressions for it. For the case of if let true = p && q {..} which also had zero regressions, we did do a in-2015 breakage, so I think the consistent thing to do would be to do this in Rust 2015 as well and thus reduce the technical debt.

Centril on 15 Sep 2018

👍2

I feel like the boat has already sailed on that. I don't really want to change the implementation again :p

mark-i-m on 15 Sep 2018

One idea might be to create a new RFC that specifies/discusses what kind of breakage is acceptable in-edition. Personally, I think the standard should be rather higher than a successful crater run + some time on nightly.

mark-i-m on 15 Sep 2018

@mark-i-m

I don't really want to change the implementation again :p

I can sympathize :)

I feel like the boat has already sailed on that.

The FCP was completed yes; (I wasn't on the lang team then...); so I might just have to live with this :(

One idea might be to create a new RFC that specifies/discusses what kind of breakage is acceptable in-edition.

An interesting thought; but sadly that would take too much time for this particular instance.

Personally, I think the standard should be rather higher than a successful crater run + some time on nightly.

I on the other hand think that a crater run that found zero regressions makes it acceptable.

Centril on 15 Sep 2018

An interesting thought; but sadly that would take too much time for this particular instance.

I meant an RFC for setting a general bar for this sort of in-edition breakage.

mark-i-m on 16 Sep 2018

Note to self: this should be documented in the edition guide somewhere... (If someone else is up for a PR on this, it might get done sooner)...

mark-i-m on 18 Sep 2018

I meant an RFC for setting a general bar for this sort of in-edition breakage.

Yeah I understood that =P

I meant an RFC for setting a general bar for this sort of in-edition breakage.

Probably worth making an issue on the guide :)

Centril on 18 Sep 2018

👍1

Documentation for when this is eventually stabilized (hopefully):

Reference: https://github.com/rust-lang-nursery/reference/pull/424
Edition Guide: https://github.com/rust-lang-nursery/edition-guide/pull/111

mark-i-m on 22 Sep 2018

I would really like to see this stabilized. =) Nominating for lang team discussion.

nikomatsakis on 5 Oct 2018

🎉3

@rfcbot fcp merge

Let's stabilize this :)

But let's also have a stabilization report with links to tests and stuff...

@rfcbot concern report-and-tests

Centril on 25 Oct 2018

🎉4

Team member @Centril has proposed to merge this. The next step is review by the rest of the tagged teams:

[x] @Centril
[x] @aturon
[x] @cramertj
[x] @eddyb
[x] @joshtriplett
[ ] @nikomatsakis
[x] @nrc
[x] @pnkfelix
[x] @scottmcm
[x] @withoutboats

Concerns:

~~report-and-tests~~ resolved by https://github.com/rust-lang/rust/issues/48075#issuecomment-433244215
~~stabilization report~~ resolved by https://github.com/rust-lang/rust/issues/48075#issuecomment-433178807

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

rfcbot on 25 Oct 2018

@rfcbot concern stabilization report

I think we need a stabilization report. This should include:

brief description of the behavior and any deviations from the RFC
which edition(s) are affected and how
links to a few tests to show the interesting aspects

@mark-i-m would you be game to produce it?

nikomatsakis on 25 Oct 2018

👍1

@rfcbot resolve stabilization report

Oh, I see I duplicated @Centril's concern.

nikomatsakis on 25 Oct 2018

❤1 😄1

Stabilization Report :tada:

(Let me know if I missed anything)

Description of behavior

Edition 2015 ? is a macro separator, not a kleene op, but using it as a separator triggers a migration lint.
- This represents no change from previous behavior except the migration lint.
Edition 2018 ? is _not_ a valid separator. ? _is_ a valid Kleene op.
- Add a Kleene operator to macros to repeat a pattern at most once: $(pat)?. Here, ? behaves like + or * but represents at most one repetition of pat.
- The ? Kleene operator does not accept any separator token, unlike + and *. For example, $(a),? is an error. This is because we don't allow trailing separators, so any separator used with the ? Kleene operator would be impossible to use.
- For example, under this proposal, $(a)?+ matches + and a+ unambiguously.
For a longer description of alternatives and behavior, see this FCP.
Note, that _the stabilization only impacts Edition 2018_.

Differences from RFC

Link to RFC

The RFC proposed a disambiguation strategy that used lookahead to discern whether a ? token was a separator for + or * OR was a ? Kleene op. We opted instead to remove ? as a separator in Edition 2018 as a breaking change, removing the ambiguity altogether.
The RFC proposed making a minor breaking change in Edition 2015 to change the disambiguation as mentioned above. Instead, we opted to only enable ? Kleene operator in Edition 2018. That is, Edition 2015 is unchanged (except for a migration lint), whereas Edition 2018 has the changes described above.
The RFC had an unresolved question: "Should the ? Kleene operator accept a separator token?". We decided the answer is "no".

Documentation and Testing

Reference: https://github.com/rust-lang-nursery/reference/pull/424
Edition Guide: https://github.com/rust-lang-nursery/edition-guide/pull/111
Test: ? _is not_ a Kleene op on _2015_
https://github.com/rust-lang/rust/blob/f498e4ec1b57c3245a2fb8e0d5d836ed56760d2d/src/test/ui/macros/macro-at-most-once-rep-2015-ques-rep.rs
Test: ? _is_ a separator on _2015_
https://github.com/rust-lang/rust/blob/f498e4ec1b57c3245a2fb8e0d5d836ed56760d2d/src/test/ui/macros/macro-at-most-once-rep-2015-ques-sep.rs
Test: ? _is_ a Kleene op on _2018_, and _not_ a separator
https://github.com/rust-lang/rust/blob/f498e4ec1b57c3245a2fb8e0d5d836ed56760d2d/src/test/ui/macros/macro-at-most-once-rep-2018.rs

mark-i-m on 26 Oct 2018

👍3

@nikomatsakis @Centril done :) Let me know if I missed anything

mark-i-m on 26 Oct 2018

Also, it looks like the checklist in the OP should be updated.

mark-i-m on 26 Oct 2018

@rfcbot resolve report-and-tests

@mark-i-m that's a most excellent report. :)

Centril on 26 Oct 2018

❤2

Well done on the super-meticulous work @mark-i-m. Glad we can finally get this feature stabilised!

alexreg on 26 Oct 2018

❤2

:bell: This is now entering its final comment period, as per the review above. :bell:

rfcbot on 20 Nov 2018

🎉4

@mark-i-m Since you did the implementation work, could you prepare a PR for the stabilization as well? :)

Centril on 23 Nov 2018

❤1

Sure, but it might be a few days before I can get to it

mark-i-m on 23 Nov 2018

❤1

@mark-i-m Ah; there's no great rush; 1.32 goes into beta on the 7th. Do you think you can get it landed by then?

Centril on 23 Nov 2018

Yeah, I think I could do it by the end of the week.

mark-i-m on 24 Nov 2018

❤1 👍1

56245 has merged

mark-i-m on 29 Nov 2018

🎉4

Rust: tracking issue for `?` macro repetition

Status

Known bugs

Unresolved questions to be answered before stabilization

Most helpful comment

All 94 comments

Stabilization Report :tada:

Description of behavior

Differences from RFC

Documentation and Testing

56245 has merged

Related issues