Skip to content

[RFC-2011] Expand matches! #110382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[RFC-2011] Expand matches! #110382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4766cf
[RFC-2011] Expand `matches!`
c410-f3r May 19, 2023
f026104
Typo
c410-f3r May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1291,6 +1291,7 @@ impl Expr {
ExprKind::Struct(..) => ExprPrecedence::Struct,
ExprKind::Repeat(..) => ExprPrecedence::Repeat,
ExprKind::Paren(..) => ExprPrecedence::Paren,
ExprKind::Matches(..) => ExprPrecedence::Matches,
ExprKind::Try(..) => ExprPrecedence::Try,
ExprKind::Yield(..) => ExprPrecedence::Yield,
ExprKind::Yeet(..) => ExprPrecedence::Yeet,
Expand Down Expand Up @@ -1488,6 +1489,9 @@ pub enum ExprKind {
/// Output of the `offset_of!()` macro.
OffsetOf(P<Ty>, P<[Ident]>),

/// Output of the `matches!()` macro.
Matches(P<Expr>, P<Arm>, P<Arm>),

/// A macro invocation; pre-expansion.
MacCall(P<MacCall>),

Expand Down
27 changes: 20 additions & 7 deletions compiler/rustc_ast/src/mut_visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ pub trait MutVisitor: Sized {
noop_visit_anon_const(c, self);
}

fn visit_arm(&mut self, arm: &mut P<Arm>) {
noop_visit_arm(arm, self)
}

fn visit_expr(&mut self, e: &mut P<Expr>) {
noop_visit_expr(e, self);
}
Expand Down Expand Up @@ -443,13 +447,7 @@ pub fn noop_visit_use_tree<T: MutVisitor>(use_tree: &mut UseTree, vis: &mut T) {
}

pub fn noop_flat_map_arm<T: MutVisitor>(mut arm: Arm, vis: &mut T) -> SmallVec<[Arm; 1]> {
let Arm { attrs, pat, guard, body, span, id, is_placeholder: _ } = &mut arm;
visit_attrs(attrs, vis);
vis.visit_id(id);
vis.visit_pat(pat);
visit_opt(guard, |guard| vis.visit_expr(guard));
vis.visit_expr(body);
vis.visit_span(span);
noop_visit_arm(&mut arm, vis);
smallvec![arm]
}

Expand Down Expand Up @@ -695,6 +693,16 @@ pub fn visit_attr_tt<T: MutVisitor>(tt: &mut AttrTokenTree, vis: &mut T) {
}
}

fn noop_visit_arm<T: MutVisitor>(arm: &mut Arm, vis: &mut T) {
let Arm { attrs, pat, guard, body, span, id, is_placeholder: _ } = arm;
visit_attrs(attrs, vis);
vis.visit_id(id);
vis.visit_pat(pat);
visit_opt(guard, |guard| vis.visit_expr(guard));
vis.visit_expr(body);
vis.visit_span(span);
}

// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
pub fn visit_tt<T: MutVisitor>(tt: &mut TokenTree, vis: &mut T) {
match tt {
Expand Down Expand Up @@ -1334,6 +1342,11 @@ pub fn noop_visit_expr<T: MutVisitor>(
vis.visit_expr(f);
visit_thin_exprs(args, vis);
}
ExprKind::Matches(expr, true_arm, false_arm) => {
vis.visit_expr(expr);
vis.visit_arm(true_arm);
vis.visit_arm(false_arm);
}
ExprKind::MethodCall(box MethodCall {
seg: PathSegment { ident, id, args: seg_args },
receiver,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_ast/src/util/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ pub enum ExprPrecedence {
InlineAsm,
OffsetOf,
Mac,
Matches,
FormatArgs,

Array,
Expand Down Expand Up @@ -329,6 +330,7 @@ impl ExprPrecedence {
| ExprPrecedence::Field
| ExprPrecedence::Index
| ExprPrecedence::Try
| ExprPrecedence::Matches
| ExprPrecedence::InlineAsm
| ExprPrecedence::Mac
| ExprPrecedence::FormatArgs
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_ast/src/visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,11 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
visitor.visit_expr(callee_expression);
walk_list!(visitor, visit_expr, arguments);
}
ExprKind::Matches(expr, true_arm, false_arm) => {
visitor.visit_expr(expr);
visitor.visit_arm(true_arm);
visitor.visit_arm(false_arm);
}
ExprKind::MethodCall(box MethodCall { seg, receiver, args, span: _ }) => {
visitor.visit_path_segment(seg);
visitor.visit_expr(receiver);
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_ast_lowering/src/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
ExprKind::InlineAsm(asm) => {
hir::ExprKind::InlineAsm(self.lower_inline_asm(e.span, asm))
}
ExprKind::Matches(expr, true_arm, false_arm) => hir::ExprKind::Match(
self.lower_expr(expr),
self.arena.alloc_from_iter(
[true_arm, false_arm].iter().map(|elem| self.lower_arm(elem)),
),
hir::MatchSource::Normal,
),
ExprKind::FormatArgs(fmt) => self.lower_format_args(e.span, fmt),
ExprKind::OffsetOf(container, fields) => hir::ExprKind::OffsetOf(
self.lower_ty(
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_ast_pretty/src/pprust/state/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,20 @@ impl<'a> State<'a> {
self.end();
self.pclose();
}
ast::ExprKind::Matches(expr, true_arm, _) => {
self.word("builtin # matches");
self.popen();
self.rbox(0, Inconsistent);
self.print_expr(expr);
self.word(", ");
self.print_pat(&true_arm.pat);
if let Some(elem) = &true_arm.guard {
self.word("if");
self.print_expr(elem);
}
self.pclose();
self.end();
}
ast::ExprKind::OffsetOf(container, fields) => {
self.word("builtin # offset_of");
self.popen();
Expand Down
44 changes: 23 additions & 21 deletions compiler/rustc_builtin_macros/src/assert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,31 +31,13 @@ pub fn expand_assert<'cx>(
// context to pick up whichever is currently in scope.
let call_site_span = cx.with_call_site_ctxt(span);

let panic_path = || {
if use_panic_2021(span) {
// On edition 2021, we always call `$crate::panic::panic_2021!()`.
Path {
span: call_site_span,
segments: cx
.std_path(&[sym::panic, sym::panic_2021])
.into_iter()
.map(|ident| PathSegment::from_ident(ident))
.collect(),
tokens: None,
}
} else {
// Before edition 2021, we call `panic!()` unqualified,
// such that it calls either `std::panic!()` or `core::panic!()`.
Path::from_ident(Ident::new(sym::panic, call_site_span))
}
};

// Simply uses the user provided message instead of generating custom outputs
let expr = if let Some(tokens) = custom_message {
let panic_path = panic_path(call_site_span, cx, span);
let then = cx.expr(
call_site_span,
ExprKind::MacCall(P(MacCall {
path: panic_path(),
path: panic_path,
args: P(DelimArgs {
dspan: DelimSpan::from_single(call_site_span),
delim: MacDelimiter::Parenthesis,
Expand All @@ -69,7 +51,8 @@ pub fn expand_assert<'cx>(
//
// FIXME(c410-f3r) See https://github.com/rust-lang/rust/issues/96949
else if let Some(features) = cx.ecfg.features && features.generic_assert {
context::Context::new(cx, call_site_span).build(cond_expr, panic_path())
let panic_path = panic_path(call_site_span, cx, span);
context::Context::new(cx, call_site_span).build(cond_expr, panic_path)
}
// If `generic_assert` is not enabled, only outputs a literal "assertion failed: ..."
// string
Expand Down Expand Up @@ -110,6 +93,25 @@ fn expr_if_not(
cx.expr_if(span, cx.expr(span, ExprKind::Unary(UnOp::Not, cond)), then, els)
}

fn panic_path(call_site_span: Span, cx: &mut ExtCtxt<'_>, span: Span) -> Path {
if use_panic_2021(span) {
// On edition 2021, we always call `$crate::panic::panic_2021!()`.
Path {
span: call_site_span,
segments: cx
.std_path(&[sym::panic, sym::panic_2021])
.into_iter()
.map(|ident| PathSegment::from_ident(ident))
.collect(),
tokens: None,
}
} else {
// Before edition 2021, we call `panic!()` unqualified,
// such that it calls either `std::panic!()` or `core::panic!()`.
Path::from_ident(Ident::new(sym::panic, call_site_span))
}
}

fn parse_assert<'a>(cx: &mut ExtCtxt<'a>, sp: Span, stream: TokenStream) -> PResult<'a, Assert> {
let mut parser = cx.new_parser_from_tts(stream);

Expand Down
15 changes: 9 additions & 6 deletions compiler/rustc_builtin_macros/src/assert/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub(super) struct Context<'cx, 'a> {
best_case_captures: Vec<Stmt>,
// Top-level `let captureN = Capture::new()` statements
capture_decls: Vec<Capture>,
cx: &'cx ExtCtxt<'a>,
cx: &'cx mut ExtCtxt<'a>,
// Formatting string used for debugging
fmt_string: String,
// If the current expression being visited consumes itself. Used to construct
Expand All @@ -41,7 +41,7 @@ pub(super) struct Context<'cx, 'a> {
}

impl<'cx, 'a> Context<'cx, 'a> {
pub(super) fn new(cx: &'cx ExtCtxt<'a>, span: Span) -> Self {
pub(super) fn new(cx: &'cx mut ExtCtxt<'a>, span: Span) -> Self {
Self {
best_case_captures: <_>::default(),
capture_decls: <_>::default(),
Expand Down Expand Up @@ -85,8 +85,8 @@ impl<'cx, 'a> Context<'cx, 'a> {

let mut assert_then_stmts = ThinVec::with_capacity(2);
assert_then_stmts.extend(best_case_captures);
assert_then_stmts.push(self.cx.stmt_expr(panic));
let assert_then = self.cx.block(span, assert_then_stmts);
assert_then_stmts.push(cx.stmt_expr(panic));
let assert_then = cx.block(span, assert_then_stmts);

let mut stmts = ThinVec::with_capacity(4);
stmts.push(initial_imports);
Expand Down Expand Up @@ -237,6 +237,9 @@ impl<'cx, 'a> Context<'cx, 'a> {
self.manage_cond_expr(prefix);
self.manage_cond_expr(suffix);
}
ExprKind::Matches(expr, _, _) => {
self.manage_cond_expr(expr);
}
Comment on lines +240 to +242
Copy link
Contributor Author

@c410-f3r c410-f3r May 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@est31 Someway, somehow it is necessary to have an indicator pointing to the scrutinee expression of matches! in order to capture its content and the use of regular declarative macros makes matches! in this context a opaque ExprKind::MacCall which can't be captured without further processing. In regards to macro 2.0, I personally don't know how it can help here.

Copy link
Member

@est31 est31 May 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah then I have misunderstood and this PR is not an use case for builtin # really. I'm sorry for having suggested it earlier.

Your PR currently hardcodes the matches! macro, meaning that it can't be shadowed or used via a different name. The same is true for offset_of! which currently isn't part of the prelude, and which is also unstable, but this PR exposes it without feature gating. Also the builtin equivalent of offset_of is not meant to be used directly as it doesnt have as good error reporting or recovery (and it tolerates some things that are caught one layer higher by the macro_rules macro).

So this is not an approach that reviewers will merge I think.

If you want to do hardcoding, you could confine it at least to what happens inside the assert!() call and just check the path of the MacCall and if it only has one item and that one is matches, then processing the content. This would also not work via shadowing and use with a different name, but would be only confined to uses inside assert!(), not everywhere, and would not touch offset_of. It would also not need a new AST item which should still be added sparingly.

That would not be perfect but many times better than what there is now in this PR.

If you want to be a bit more advanced you could maybe try to resolve the macro first, then check if it is the same SyntaxExtension as the one of a pre-stored matches syntax extension.

The correct way of using builtin # here is to both turn matches and assert into builtin # constructs, but personally I'm more of a fan of re-parsing the same info multiple times.

Copy link
Contributor Author

@c410-f3r c410-f3r May 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @est31

Hum... Looks like a way forward is unclear, three different approaches were already presented (including adhoc processing inside assert as you commented) without much apparent success.

To avoid further back-and-forth, I think it is better to wait for @petrochenkov. Personally, I am happy with any strategy as long as matches! are expanded :)

If you want to be a bit more advanced you could maybe try to resolve the macro first, then check if it is the same SyntaxExtension as the one of a pre-stored matches syntax extension.

I hadn't thought that, looks promising!

ExprKind::MethodCall(call) => {
for arg in &mut call.args {
self.manage_cond_expr(arg);
Expand Down Expand Up @@ -295,17 +298,17 @@ impl<'cx, 'a> Context<'cx, 'a> {
| ExprKind::Continue(_)
| ExprKind::Err
| ExprKind::Field(_, _)
| ExprKind::FormatArgs(_)
| ExprKind::ForLoop(_, _, _, _)
| ExprKind::FormatArgs(_)
| ExprKind::If(_, _, _)
| ExprKind::IncludedBytes(..)
| ExprKind::InlineAsm(_)
| ExprKind::OffsetOf(_, _)
| ExprKind::Let(_, _, _)
| ExprKind::Lit(_)
| ExprKind::Loop(_, _, _)
| ExprKind::MacCall(_)
| ExprKind::Match(_, _)
| ExprKind::OffsetOf(_, _)
| ExprKind::Path(_, _)
| ExprKind::Ret(_)
| ExprKind::Try(_)
Expand Down
Loading