rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.89k stars 12.52k forks source link

Non-terminals, e.g. expr, have diverged between parser and macro matcher #86730

Open pnkfelix opened 3 years ago

pnkfelix commented 3 years ago

Spawned off of PR #84364, based on discussion from lang team meeting (minutes, youtube).

The meaning of the non-terminal "expr" according to the rustc parser has changed (or would like to change) from its meaning according to the current rustc macro matcher.

Concretely:

In both cases, for backwards compatibility, the expr fragment specifier does not accept these new productions. Examples of the kinds of problems this causes follow:

In other words, today you simply cannot write a macro fragment specifier denoting expressions that will match const { EXPR } or let PAT = EXPR, other than something like $e:tt, which would also match arbitrary token-trees, which won't catch errors as effectively as a more precise fragment specifier.

So, the main question this raises is: How should we resolve this divergence between the parser and macro-rules matcher?

pnkfelix commented 3 years ago

Also, lang team discussion has concluded that we don't have to solve this problem until one of inline_const (#76001) or let_chains (#53667) is being stabilized.

petrochenkov commented 3 years ago

It's not only expr, other matchers may be affected too. The matcher logic needs to be audited and implemented in terms of "reused parser code" + "explicit exceptions for backward compatibility", instead of the inconsistent ad hoc mess that is used now. UPD: At least expr and ty use can_begin_expr/can_begin_type from the parser now, IIRC it was worse in the past.

The "explicit exceptions for backward compatibility" part can be made conditional on an edition, but that's a technicality.

bstrie commented 2 years ago

Another option is to do the maximally backwards-compatible thing with macros 1.0 (either nothing at all, or introduce a unique fragment specifier), and then fix this properly in macros 2.0 while they're still unstable.

bstrie commented 2 years ago

Worth noting that this appears to be the one and only remaining issue blocking the stabilization of inline_const, which itself is the only remaining issue blocking the stabilization of asm_const, so getting this sorted out would be very impactful.

memoryruins commented 2 years ago

@pnkfelix

Also, lang team discussion has concluded that we don't have to solve this problem until one of inline_const (https://github.com/rust-lang/rust/issues/76001) or let_chains (https://github.com/rust-lang/rust/issues/53667) is being stabilized.

The stabilization proposal for let_chains is currently entering its final comment period https://github.com/rust-lang/rust/pull/94927#issuecomment-1182764818. I believe its macro concern in https://github.com/rust-lang/rust/pull/94927#issuecomment-1142439496 was resolved by forbidding let in certain places in https://github.com/rust-lang/rust/pull/97295 (unless if there was another change that helped resolved it).

Would it be possible to make a similar change to unblock inline_const? and/or now that let_chains is in the process of being stabilized, should this issue be prioritized?

cc @joshtriplett since you opened/resolved the macro concerns on let_chains stabilization, opened an FCP for a different asm feature recently, and are lang team :)

joshtriplett commented 2 years ago

Since let PAT = expr isn't allowed in an arbitrary expression, only in if let and while let, it sholdn't be added to expr. And AFAICT you currently have to match if-let and while-let explicitly in macro matches if you want their contents. So, people would need to add support for let chains in those matches. We could add something like cond to help with that, but I don't think that's a blocker.

RalfJung commented 1 year ago

@rust-lang/lang https://github.com/rust-lang/lang-team/issues/111 has been closed, so what is the plan to make progress here? This seems to be one of the last blocking issues for inline const expr.

TimNN commented 1 year ago

Would a lint that triggers whenever an :expr matcher does not match a const { ... } be maybe sufficient to allow stabilizing inline_const?

That wouldn't change the behavior of existing macros and the behavior for existing macros is reasonable, IMO:

If a macro currently has rules that explicitly match const $x:expr then:

If a macro does not have rules that explicitly match const $x:expr then:

Whether an :expr2024 matcher is needed or whether the behavior of :expr should change with the next edition could be resolved at some later point.

traviscross commented 10 months ago

Further action here is now gated on acceptance of rust-lang/rfcs#3531, "Macro matcher fragment specifiers edition policy".

RalfJung commented 6 months ago

Further action here is now gated on acceptance of rust-lang/rfcs#3531, "Macro matcher fragment specifiers edition policy".

That RFC has been accepted. Elsewhere, @pnkfelix wrote

re todo item "resolve expr fragment specifier issue", see also https://github.com/rust-lang/rfcs/pull/3531, "Macro matcher fragment specifiers edition policy".

So -- what does that RFC being accepted mean for this issue, and for inline-const? Can the issue be closed? Can it be removed from the "blocking" list of inline consts?

traviscross commented 5 months ago

The acceptance of rust-lang/rfcs#3531 means that there's now a policy for how to handle this, but someone (help wanted, cc @rust-lang/wg-macros) needs to apply this policy to macro fragment specifiers that have diverged from the grammar, e.g. expr, as discussed above.

iamahuman commented 5 months ago

The acceptance of rust-lang/rfcs#3531 means that there's now a policy for how to handle this, but someone (help wanted, cc @rust-lang/wg-macros) needs to apply this policy to macro fragment specifiers that have diverged from the grammar, e.g. expr, as discussed above.

How about reviving PR #84364? Minor changes (e.g., s/expr202x/expr_2021/g; s/expr2015/expr_2015/g) should be enough to make it compliant, I think.

RalfJung commented 5 months ago

The acceptance of rust-lang/rfcs#3531 means that there's now a policy for how to handle this, but someone (help wanted, cc @rust-lang/wg-macros) needs to apply this policy to macro fragment specifiers that have diverged from the grammar, e.g. expr, as discussed above.

Okay, thanks.

Is this a blocker for https://github.com/rust-lang/rust/pull/104087 (stabilizing inline const) or something that can also be done after stabilization?

eholk commented 5 months ago

Is this a blocker for https://github.com/rust-lang/rust/pull/104087 (stabilizing inline const) or something that can also be done after stabilization?

I wouldn't think this needs to block stabilizing inline const; if we don't get it ahead of time it just means expr will be a little weird for a little bit.

I've been working with @vincenzopalazzo to implement the RFC. We had a first PR out at #123865.