Syntactically reject equality predicates

[fmease]

View all comments

Background (History)

Equality predicates have been syntactically valid since PR #39158 / nightly-2017-02-02 / 1.16 (released 2017-03-16, 9 years ago), both forms that is: $ty = $ty and $ty == $ty. They're not registered as an unstable feature despite having a tracking issue (#20041). Naturally, they don't have a feature gate. Of course, we reject them post-expansion, so they are still semantically invalid.

Parser scaffolding for $ident = $ty was added in RUST-19391 (2014) which was then generalized to $ty = $ty in RUST-20002 (2014) and extended to additionally cover $ty == $ty in RUST-39158 (2017). As mentioned, the last PR also made them grammatical.

RUST-87471 (2021) attempted to start impl'ing typeck'ing but it was closed due to concerns: #87471 (comment) (already back in 2017 there were concerns: https://github.com/rust-lang/rust/pull/39158/changes#r97811244).

In 2022, T-lang discussed this feature during a meeting and raised concerns: #20041 (comment). However, they were inclined to accept a restricted form, namely T::AssocTy = $ty (T is a (self) ty param or a self ty alias) and <$ty as Trait>::AssocTy = $ty since that's trivial to support in HIR ty lowering (this is still accurate).

Change

This renders equality predicates $ty = $ty and $ty == $ty syntactically invalid again. On stable, they're merely semantically invalid. This is motivated by the fact that

1. their syntax isn't even decided upon
   1. should it be = or ==?
   2. should the LHS be syntactically restricted to (possibly qualified) paths (i.e., TypePath | QualifiedPathInType), only semantically or not at all?
      • "syntactically" could indeed make sense since we might want to generalize types to so-called terms here (i.e., Type | GenericArgsConst) for mGCA but we can't generalize $ty = $ty to $term = $term due to ambiguities ¹ that would require backtracking whereas $typepath = $term wouldn't have that problem
2. we might (decide to) never impl this feature
   • after all, implementation concerns were raised as early as 9 years ago which are still relevant today even with the next-gen trait solver on the horizon
3. the compiler + tools contain a bunch of code for dealing with these predicates
   • like lowering, name resolution, rustfmt and Clippy
   • that's entirely useless since they get unconditionally rejected anyway
   • this code doesn't need to exist (for now)

I've merely upgraded the semantic hard error to a syntactic hard error, I've intentionally not introduced an unstable feature under which equality predicates become legal.

That's because a hard error is easier to push through procedurally, it's an important first step (we can always turn it into a feature gate error later on) and since this potential feature isn't backed by any official lang experiment.

I don't consider T-lang's message #20041 (comment) from 2022 to be sufficient because it doesn't answer questions like (1.i), (1.ii) or whether code like T: Trait<A>, <T as Trait<B>>::AssocTy = Ty (A ≠ B) or T: Trait<'r>, for<'a> <T as Trait<'a>>::AssocTy = Ty should be legal (which would be more powerful / could lead to typechecking issues, idk) or not assuming we're going with the restricted form ofc (we would need input from T-types).

Lang nomination

#153513 (comment)

Footnotes

1. Code lile fn f() -> i32 where { 0 } is intentionally legal today but if we had $term = $term the { 0 } could then also be the start of an equality predicate like { 0 } = 0. ↩

[fmease]

@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3248bac (3248bace201b6227b0ff78eacc8b00d805551a86, parent: ff086354c9fc93e1da1d2f4d255456624fbcd099)

[fmease]

@craterbot check

[craterbot]

👌 Experiment pr-153513 created and queued.
🤖 Automatically detected try build 3248bac
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-153513 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-153513 is completed!
📊 6 regressed and 2 fixed (839795 total)
📊 2542 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-153513/retry-regressed-list.txt ↩

[fmease]

@craterbot run mode=check-only crates=https://crater-reports.s3.amazonaws.com/pr-153513/retry-regressed-list.txt p=1

[craterbot]

👌 Experiment pr-153513-1 created and queued.
🤖 Automatically detected try build 3248bac
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[jackh726]

I did intend to submit a follow-up that actually removes code but I didn't do so so far since I didn't know yet if T-lang/T-types would like to officially endorse this as an in-tree experiment, so I've saved my efforts.

Ah okay. I mean, I don't think we should rip out this code entirely. But what that means, I think I'll elaborate below.

I do know that very well. I have linked to #20041 (comment) twice from the PR description. I know that that's a lot easier to support.

Well

1. I don't want to do anything that will make T::Asocc = U harder, at all.
2. I'd personally be generally in favor of not making our life harder in some theoretical future that we want to try to implement generalized T = U, but I know that if this ever happens, it'll be far in the future. And it's possible that we might even limit it to so form like <path> = <path> or <param> = <path>, or <path> = <term>. So idk on this one.

By syntactically accepting this today, what code are we allowing or preventing today that helps us in the future?

I think it would be good to have specific tests and code examples to talk about this concretely. Specifically: if we change this to syntactically reject, is there new code (likely in macros) that will start to compile that stops working with an added feature gate?

2. Moreover, as I've explained in said section, it would be _very advantageous_ if we changed the grammar from `$ty = $ty`to `$path_or_qpath = $ty`  (modulo `=`, `==`) which we can then _unstably_ generalize to `$path_or_qpath = $term` (_term_ is [mGCA](github.com/rust-lang/rust/issues/132980)-lingo for _Type | GenericArgsConstArg_) to mirror `$traitref<$pathseg = $term>` (e.g., `Trait<CT = 0>`, and associated const binding; formerly under feature `associated_const_equality`, now under feature `min_generic_const_args`).
   
   * If we went with `$type = $type` (mod …)  we couldn't generalize it to `$term = $term` due to ambiguities / the need to backtrack; copy/pasting example: Code like `fn f() -> i32 where { 0 }` is intentionally legal today but if we had `$term = $term` the `{ 0 }` could then also be the start of an equality predicate like `{ 0 } = 0`.

We never are required to generalize to $term = $term, we could instead (in theory) allow any of $ty = $ty OR $path_or_qpath = $term, for example.

There are no downsides!

I either am overthinking, or I'm missing something. Maybe I need to read back through why this originally was syntactically accepted but semantically rejected, and what the motivation of that was.

[fmease]

Thanks for your reply!

There are no downsides!

I either am overthinking, or I'm missing something. Maybe I need to read back through why this originally was syntactically accepted but semantically rejected, and what the motivation of that was.

RUST-39158 (from 2017) is the PR that made equality predicates syntactically well-formed … next to much larger changes. See it for yourself! Still, I'd like to remark that this change isn't mentioned in the description of said PR and not explained in its comments. On the contrary, in this PR Niko even wrote in a review comment that we should remove parsing support for equality predicates (i.e., making them syntactically malformed) once the PR is merged all the while the PR made them syntactically legal in the first place!

I consider this to be nothing but a historical oversight. That's why I've labeled this PR with I-easy-decision and why I'm so surprised by the reception ^^'.

[fmease]

I do know that very well. I have linked to #20041 (comment) twice from the PR description. I know that that's a lot easier to support.

Well

1. I don't want to do anything that will make T::Asocc = U harder, at all.
2. I'd personally be generally in favor of not making our life harder in some theoretical future that we want to try to implement generalized T = U, but I know that if this ever happens, it'll be far in the future. And it's possible that we might even limit it to so form like <path> = <path> or <param> = <path>, or <path> = <term>. So idk on this one.

The removal of the AST & HIR node, the AST lowering & name resolution code doesn't need to happen. I'd already be beyond ecstatic if we fixed this historical mistake and rendered them syntactically invalid again, I ask nothing more. Whether I'm sending such a removal PR is entirely dependent on T-lang's/T-types's general vibes … and since you as a T-types member are still interested in some form of this feature eventually I'm happy to keep that code.

However, I'd like to say that deleting all this code shouldn't make it harder for a future contributor to implement this feature; the code that's currently in place is extremely trivial, it's just matches & node visits. Grep for word EqPredicate yourself (e.g., rg -w EqPredicate -C5 compiler). Nothing valuable would be lost.

[fmease]

By syntactically accepting this today, what code are we allowing or preventing today that helps us in the future?

In short:

1. Today, it enables iDSL writers to (start to) depend on the syntax.
2. In a tomorrow where typechecking for them has been implemented, it would inevitably lead to some widely used crates starting to use said syntax behind #[cfg(feature = "nightly")] (an age-old trick) which would lead to us breaking the code of tons of stable users if we ever changed the syntax in some way (or removed it entirely) (most likely us dropping one of =, == in favor of the other).

I'll further explain this in a moment.

Important

These two points obviously apply to all unstable syntax and they are precisely the reason why we've migrated to pre-expansion feature gates from post-expansion ones. They are not hypotheticals!

In detail:

Reason (1)

Libraries might use this syntax as part of their (internal) domain-specific language. The following code would break if we dropped $ty = $ty or $ty == $ty (both or each in favor of the other):

#[krate::domain_specific_language]
fn f() where policy = read_only, context = Context { /* ... */ }
#[other_krate::other_domain_specific_language]
fn g() where capabilities == (diverge, IO) { /* ... */ }

Evidently, this is a theoretical example. Crater couldn't find any regressions but of course that doesn't mean nobody is relying on it. I know they've been legal for 9 years but we should still reject them sooner than later to avoid the situation where somebody starts relying on this existing. Continuing to allow this syntax despite of that is unprincipled in my eyes. It's such an easy change / fix on top of that. As it stands it's officially part of the grammar of (stable) Rust.

Reason (2)

Let's assume it's year 2027, T-lang + T-types have approved an in-tree experiment and somebody has implemented the type checking part (or rather the HIR ty lowering). We have to assume that the newly added feature is post-expansion gated (unlike the majority of unstable features).

If it were pre-expansion gated it would mean T-lang approved equality predicates becoming ungrammatical again (that's what pre-expansion gating means) but did not approve this PR which would be highly contradictory, nonsensical, unfair even.

So, it's a post-expansion feature gate. Consequently, it's basically inevitable that crates will start using the infamous "#[cfg(feature = "nightly")]" trick for this feature: They innocently and for the sake of goodwill want the crate to be usable by stable users but also offer "nightly extensions" for nightly users:

// Hypothetical future where unstable feature `equality_predicates` exists.
// This is a widely used library crate with 100s if not 1000s of *stable* users!
#![cfg_attr(feature = "nightly", feature(equality_predicates))]

// ... stable API surface ...

#[cfg(feature = "nightly")]
pub fn extra_functionality<T: Iterator>() where T::Item = i32 { /* ... */ }

#[cfg(feature = "nightly")]
pub fn more_extra_functionality<T: Iterator>() where () == T::Item { /* ... */ }

// ... stable API surface ...

Again, we'll incredibly likely drop = or == in favor of the other eventually (or, less likely so, we might decide to remove the feature entirely). What would that mean for the hundreds or thousands of hypothetical downstream users that are on a stable toolchain? That's right, it would break all of their code.

This is not a made-up scenario, there's a reason why we still have legacy(!) post-expansion gated feature (specialization, negative impls, try blocks, box patterns, trait aliases, auto traits, syntax impl Trait for .. {} and decl macros), cc #154045. The crater runs reported tons of breakages.

I can also speak from personal experience: I once tried to remove the auto trait Trait {} syntax (only keeping an internal feature, #[rustc_auto_trait] trait Trait {} for use in the stdlib). See this HackMD. This proposal has failed. E.g., leptos-rs/leptos (11.9k stars at that time) and PyO3/pyo3 (9.4k stars at that time) depended on that syntax behind #[cfg(feature = "nightly")]! Removing the unstable syntax would've meant breaking thousands of stable users!

[fmease]

Specifically: if we change this to syntactically reject, is there new code (likely in macros) that will start to compile that stops working with an added feature gate?

I can't of any such scenario. My PR will simply reject more programs, so we have more leeway in designing this dormant feature properly in the future.

You are probably alluding to the fact that the matching behavior of decl macros plus macro fragment specifiers is an infamous forward compatibility hazard for us. E.g.,

macro_rules! check {
    ($ty:ty) => { compile_error!("type detected!"); };
    ($($tt:tt)*) => {};
}

check!(/* PREFIX ... */); // <- this will fail if we add a new kind of type
                          //    that has single-token prefix PREFIX.
                          //    (in the compiler: if we extend `Token::can_begin_type` to contain PREFIX) 

Since there (thankfully) isn't a fragment specifier that corresponds to bounds or predicates, no such cases should exist.

Indeed, = is in the follow-set of type fragments but arms like $a:ty = $b:ty wouldn't start getting rejected with a feature gate error assuming this PR was merged before the feature was introduced because they're not a predicate. Only if we had $p:predicate that would start getting problematic. $i:item isn't problematic because predicates are never part of the prefix of items.

[jackh726]

Rather than trying to go back and forth on github (since, I'm coming back and forth to this), I opened a zulip topic: #t-lang > Synatically reject equality predicates #153513 @ 💬

[rust-bors]

☔ The latest upstream changes (presumably #154160) made this pull request unmergeable. Please resolve the merge conflicts.

[tmandry]

Like Jack, I was skeptical because I thought this would make it harder to introduce these in the future. But since we don't have any macro matchers today that can match over a where clause / bound, it won't do that.

@rfcbot fcp merge lang

[rust-rfcbot]

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

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[traviscross]

Thanks @fmease for pushing this forward; thanks @jackh726 for looking into this.

@rfcbot reviewed

[rust-rfcbot]

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

[nikomatsakis]

@rfcbot reviewed

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.