[WIP] Check future proofing of macros with multiple arms using FIRST sets.

[LeoTestard]

Far from being finished. The first_sets_disjoints is still the biggest missing piece.
cc @pnkfelix @nikomatsakis

[rust-highfive]

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

[durka]

I like the warnings, but I think we should use this info to be careful about syntax changes, rather than punishing macro authors for not predicting the future (so I would be against hard errors).

[nikomatsakis]

I imagine we would try to take the effect of such changes into account
when making decisions, but I wouldn't consider it a decisive factor.
Consider that just about any change to the grammar can affect some
program somewhere, and we can't actually know what the full set of
programs do. Hopefully we'll find a way to employ rules that actually
prevent "future-hostile" macros but in the meantime warnings would
surely be an improvement.

On Wed, Jun 08, 2016 at 11:41:37AM -0700, Alex Burka wrote:

I like the warnings, but I think we should use this info to be careful about syntax changes, rather than punishing macro authors for not predicting the future.

---

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#34140 (comment)

[LeoTestard]

Ok so here are the first results (including the Rust codebase and the unit tests, not crater yet):

Broken macros in the Rust codebase:

• libcollections::vec (macros.rs)
• libsyntax::declare_features (feature_gate.rs)
• libsyntax::maybe_whole (parser.rs)
• librustc::enum_from_i32 (macros.rs)
• librustc::hir_vec (hir)
• librustc_metadata::enum_from_i32 (macros.rs) (dup of librustc::enum_from_i32?)
• librustc_mir::unpack (build)
• librustc_trans::ifn (context.rs)

Failed tests:

• compile-fail/issue-16098.rs
• compile-fail/macro-follow.rs
• compile-fail/macro-followed-by-seq-bad.rs
• compile-fail/macro-input-future-proofing.rs

For now I identified three patterns:

• Legit failures
• Failures due to the fact that the analysis currently does not descend into sequence repetitions, like vec!, but on macros that otherwise look legal
• T versus NT failures, where T has the priority (being in the first arm), but rejected because NT could match several token trees and we don't know where we should continue the analysis. However, if the rest of the first matcher after T doesn't contain any NT matcher, then I think it's not necessary to continue the analysis and we can just accept the macro (or reject it, based on the previous context, but it should accept the cases we have here).

cc @pnkfelix

[bors]

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

[bors]

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

[alexcrichton]

r? @pnkfelix

Would be good to also get a resolution for #33840 as well.

[brson]

Ditto. Let's close if this isn't going to progress.

[steveklabnik]

Ditto. Let's close if this isn't going to progress.

Agreed. @LeoTestard if you end up updating this, please let us know, but with months with no activity, I'm going to give this a close.