syntax: Trait object types permit extra parentheses on the first bound

[petrochenkov]

// Accepted while it should not.
type A = Box<(Trait1) + Trait2 + 'lifetime>;

// Correct syntax.
type A = Box<Trait1 + Trait2 + 'lifetime>;

The syntax for object types is the same as for other bounds - Bound + Bound + Bound + .... and parentheses are not a part of bound syntax.

This is a stable-stable regression, introduced in Rust 1.6 by #29870.
cc #39169 #39179

Current status

• Refactor parsing of trait object types #40043 - (Bound) + Bound + .... is still parsed , but with a warning. This is a "hardcoded" warning and not a lint, lints cannot be reported from libsyntax.
• PR ? turns this syntax into a hard error

[brson]

Needs a P-tag. Thanks for the fixes @petrochenkov

[nikomatsakis]

I'm removing Nominated -- it's not clear that we want to actually enforce this rule. See the discussion on #40043

[nikomatsakis]

Nominating for lang-team discussion: there was some back and forth on the original PR. We should figure out what is the right venue/process to finalize decision here.

[nikomatsakis]

OK, we talked about this in @rust-lang/lang meeting. We decided that it'd be best to have the discussion stay on this issue, and we can just do a "rfcbot" final decision here when needed.

I think there was also some agreement @petrochenkov that it makes sense to support Box<'a + Trait>, contra to what I said earlier, primarily because it'd be nice if things you can type in where-clauses -- to the extent possible -- also work in types (the exception being that T: 'a is a valid bound in a where-clause, but Box<'a> cannot not be parsed as a type).

Nonetheless there was some reluctance to break code that is using parentheses. It's not entirely clear why that should be required. So one question that @pnkfelix wanted to clarify was: Can you elaborate on why you would not want to permit () in bounds? In an earlier comment, you wrote:

I'm afraid these parens will become a dark corner of the language used only in interview questions.

Is it simply that there are no other operators in where-clauses, and hence supporting parentheses is sort of unnecessary?

(Put another way, the idea of allowing all bounds to accepts parens is appealing and seems natural enough.)

[petrochenkov]

Can you elaborate on why you would not want to permit () in bounds?

Ok, I'll try to elaborate.
There are two positions from which this issue can be resolved - 1) these parens are needed only for backward compatibility and 2) these parens are legitimately useful.

[petrochenkov]

I. The parens are only for backward compatibility and not useful enough on their own.

Assume for a minute that the 1.6 regression never happened, the parens are not accepted, and I'm submitting a new RFC proposing to add parentheses to the bounds grammar.
If you'd tend to reject that RFC as insufficiently motivated, you are on this first position.

The best solution from this position is what I did in #40043 originally - continue parsing the parens around the first bound, as they are parsed now, but issue a warning. The warning can be turned into a hard error eventually, maybe in half a year, maybe later, it's not especially important.

[petrochenkov]

II. The parens are legitimately useful.

It may be reasonable to visually emphasize that Fn(A) -> B + Send is grouped as (Fn(A) -> B) + Send even if it's not technically necessary and looks a bit more noisy.

Nicer grouping is still not a very strong motivation, so even if we add it, we should not sacrifice other important properties.
I.e. syntactic disambiguation GENERIC_BOUNDS (aka TRAIT_OBJECT) vs ANY_OTHER_TYPE should still be as simple as possible, and additions to the syntax in general should be minimized.

Now let's explore some possible syntax additions.

1. (an::<arbitrary>::path) - ACCEPT, if (Fn(A) -> B) is accepted, then other paths in parens should be accepted, it's simpler and it would be strange to discriminate on the basis of syntax used for generic arguments inside of the path.
2. ((an::<arbitrary>::path)) - REJECT, extra parens are useless (even harmful) for visual grouping and complicate disambiguation, now you need to loop through all the parens to find out if it's a trait object or not.
3. (for<'a> Fn<'a>(A) -> B) - NOT SURE (ACCEPT), may be useful for grouping, complicates disambiguation - more common prefixes between trait objects and other types.
4. for<'a> (Fn<'a>(A) -> B) - NOT SURE (REJECT), it's probably better to group for together with the remaining poly-trait (3.).
5. (for<'a> (Trait<'a>)) - REJECT, one parens (probably the outer ones) are enough for grouping and we are trying to minimize grammar additions.
6. ('a) - REJECT, grouping is useless for lifetimes, complicates disambiguation - you have to add logic for rejecting ('a) but accepting ('a) + Trait in type position.
7. ?(Sized), (?Sized), ?for<'a> (Sized), ?(for<'a> Sized), (?for<'a> Sized), (?for<'a> (Sized)), (?(for<'a> Sized)), (?(for<'a> (Sized))) - NOT SURE, all of these are extremely useless and go against the goal of minimizing new syntax, and some complicate disambiguation. I'd say (?WHATEVER) without inner parens should be accepted if 3. is accepted for consistency and because it doesn't complicates disambiguation further.

[petrochenkov]

Ok, now I spent more than an hour of time fleshing out the details of this completely worthless feature.
I don't want neither the lang team, nor anyone else to waste their time in the same way.
That's why I proposed to just reject the parens and forget about this "problem" forever.

[joshtriplett]

Until I read #39318 (comment) , I agreed with the first position: only useful for backward compatibility. However, I find your example of Fn quite compelling. Almost any combination of Fn... + AnotherTrait gets clearer with parentheses grouping the whole Fn together. Thank you for your detailed comment and analysis.

Based on that comment, in the absence of some hard conflict with another grammar feature, I'd be tempted to say "keep parentheses, and don't deprecate them". I'd even be inclined to say that the style guide should recommend them around Fn types when used in combination with +.

[withoutboats]

@petrochenkov I agree we shouldn't spend too much time on this; I don't even want to spend the time to track a deprecation. That's why I want to just accept parens in bounds and be done with it.

There are a number of quirks that we accept for backwards compatibility reasons (especially in bounds syntax, such as trailing +). I'm okay with this, none of them seem surprising to accept or like they mean things you wouldn't expect. Maybe if we have 'epochs' we can clear house at the next epoch, but for now I'm inclined to just accept these.

[brson]

Seems like lang team has an idea what to do here. Let's try to resolve quickly one way or the other.

[petrochenkov]

@withoutboats

I don't even want to spend the time to track a deprecation. That's why I want to just accept parens in bounds and be done with it.

Deprecation is certainly easier (add couple of lines of code + test, then remove them) and affects only rustc.
New syntax affects all the tooling and documentation, rustfmt will have to support it, IDEs will have to support it, people seeing it for the first time will have to learn it. These are all costs.

[withoutboats]

@petrochenkov A parsing impl needs to support it and the reference needs to mention it, but paren grouping of operations is not a very foreign concept to most users (if it is they'll have to learn it anyway for other uses) & we don't ever recommend using it. I really don't see these as significant costs.

Deprecation of course does not only impact rustc but it also impacts any users who were using parens in their trait objects. Do we have crater results on that?

Its also plausible that we will create bound operators someday other than + and order of operations will start to matter, forcing us to reintroduce this.

I don't feel strongly about this and it seems like you do, so a part of me is inclined to just say we should do whatever.

[petrochenkov]

Do we have crater results on that?

#40043 (comment) - three regressions, all are fixed upstream.

[nikomatsakis]

@petrochenkov it feels funny to me to support parens but not ((((B))))) -- the latter could be useful e.g. when generating code, where sometimes extra parens are convenient.

[nikomatsakis]

As for your other examples:

for<'a> (Fn<'a>(A) -> B)

I would reject this for now, but I could see eventually that it might be useful to be able to share the quantifier over multiple bounds (e.g., for<T> (Foo: Bar<T> + Foo: Baz<T>)). Certainly a chalk-like abstraction makes this quite natural.

[petrochenkov]

@nikomatsakis

Every it feels funny to me to support parens but not ((((B))))

Every syntactic addition in this area adds more ambiguities and I'd want to avoid them unless really necessary.
Things would be so much simpler with dyn Trait.

[nikomatsakis]

I think I'd really prefer to agree to a particular grammar here. It seems surprising to me to allow some forms of bounds to be parenthesized but not others -- e.g. (Send) + Sync but not ('a) + Sync. Can you say more about your concerns with parenthesized lifetimes? I thought we agreed that (e.g.) it's not a problem that Box<'a +> would be parsed as a type (and rejected later)...?

I also don't have a problem with (Send+) + Sync parsing, given that we support trailing separators, and that I would expect (Send + Sync) + Foo to parse.

[petrochenkov]

@nikomatsakis

It seems surprising to me to allow some forms of bounds to be parenthesized but not others -- e.g. (Send) + Sync but not ('a) + Sync

Disambiguation code just continues piling up when you add ('a) or other syntax.
Given that this code has zero practical benefit, only maintenance cost, I'd really want to avoid adding it.
Can we abandon all these discussions and go the deprecation route already? So much hassle over nothing.