MBE: more checks at definition time

[mark-i-m]

There are a few checks that are currently only done at macro invocation time (or are not done at all) that really would make more sense to do a macro definition time.

This is a tracking issue to implement these checks.

• Checking that the Kleene operator used in the body is the same as that in the matcher (Add meta-variable checks in macro definitions #62008)

  • https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=b440f281efd992b9522275950e5689c2

• Checking that all meta-variables used in the body are defined and in scope (Add meta-variable checks in macro definitions #62008)

  • https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2d569d0d34213bbab2f1f2a9485546fd

• Checking that all meta-variables used in the body are repeating at the right depth (Add meta-variable checks in macro definitions #62008)

  • https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=c8adccf18462bb0955e786ceb1fdd1b7

• Checking that the body of a macro is actually valid rust syntax (to the extent that we can, anyway)

  • https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=55fee22139ff7ef1f9470cd048e62885
  • This might be impossible... leaving it off for now...

• Maybe fix false positives/negatives

• Possibly propose not using kleene op on RHS

[mark-i-m]

Could someone tag this with cleanup and macros and help-wanted please?

EDIT: and tracking-issue

[mark-i-m]

cc @petrochenkov

[jonas-schievink]

@mark-i-m you can apply labels yourself by using @rustbot modify labels

[mark-i-m]

Oh, that's good to know. Thanks :)

@rustbot modify labels: +C-cleanup

[mark-i-m]

Just some a quick note for anyone who wants to implement this: the checks should probably be inserted here, where there are already some other checks:

rust/src/libsyntax/ext/tt/macro_rules.rs

Lines 356 to 369 in dec4c52

	for rhs in &rhses {
	valid &= check_rhs(sess, rhs);
	}
	// don't abort iteration early, so that errors for multiple lhses can be reported
	for lhs in &lhses {
	valid &= check_lhs_no_empty_seq(sess, &[lhs.clone()]);
	valid &= check_lhs_duplicate_matcher_bindings(
	sess,
	&[lhs.clone()],
	&mut FxHashMap::default(),
	def.id
	);
	}

You can look at the existing checks to see what they are like.

Also, anything implemented will probably need a crater run. I think breakage here is acceptable, since any code that does the things in the OP is almost certainly wrong.

EDIT: also, we would probably need to follow a procedure like #57742 and its PRs.

[mark-i-m]

@rustbot modify labels: +E-medium +E-mentor