Skip to content
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.

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

AST pretty: Use builtin_syntax for type ascription #124637

Merged
merged 1 commit into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 10 additions & 4 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1390,7 +1390,7 @@ pub struct StructExpr {
// Adding a new variant? Please update `test_expr` in `tests/ui/macros/stringify.rs`.
#[derive(Clone, Encodable, Decodable, Debug)]
pub enum ExprKind {
/// An array (`[a, b, c, d]`)
/// An array (e.g, `[a, b, c, d]`).
Array(ThinVec<P<Expr>>),
/// Allow anonymous constants from an inline `const` block
ConstBlock(AnonConst),
Expand All @@ -1401,7 +1401,7 @@ pub enum ExprKind {
/// This also represents calling the constructor of
/// tuple-like ADTs such as tuple structs and enum variants.
Call(P<Expr>, ThinVec<P<Expr>>),
/// A method call (e.g. `x.foo::<Bar, Baz>(a, b, c)`).
/// A method call (e.g., `x.foo::<Bar, Baz>(a, b, c)`).
MethodCall(Box<MethodCall>),
/// A tuple (e.g., `(a, b, c, d)`).
Tup(ThinVec<P<Expr>>),
Expand All @@ -1413,7 +1413,10 @@ pub enum ExprKind {
Lit(token::Lit),
/// A cast (e.g., `foo as f64`).
Cast(P<Expr>, P<Ty>),
/// A type ascription (e.g., `42: usize`).
/// A type ascription (e.g., `builtin # type_ascribe(42, usize)`).
///
/// Usually not written directly in user code but
/// indirectly via the macro `type_ascribe!(...)`.
Type(P<Expr>, P<Ty>),
/// A `let pat = expr` expression that is only semantically allowed in the condition
/// of `if` / `while` expressions. (e.g., `if let 0 = x { .. }`).
Expand Down Expand Up @@ -1488,7 +1491,10 @@ pub enum ExprKind {
/// Output of the `asm!()` macro.
InlineAsm(P<InlineAsm>),

/// Output of the `offset_of!()` macro.
/// An `offset_of` expression (e.g., `builtin # offset_of(Struct, field)`).
///
/// Usually not written directly in user code but
/// indirectly via the macro `core::mem::offset_of!(...)`.
OffsetOf(P<Ty>, P<[Ident]>),

/// A macro invocation; pre-expansion.
Expand Down
15 changes: 8 additions & 7 deletions compiler/rustc_ast_pretty/src/pprust/state/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,8 @@ impl<'a> State<'a> {
self.print_type(ty);
}
ast::ExprKind::Type(expr, ty) => {
self.word("type_ascribe!(");
self.word("builtin # type_ascribe");
Copy link
Member

Choose a reason for hiding this comment

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

Hmm... I think I'm kind of torn here. I feel like it's still useful to print it like type_ascribe!(, though it's kind of lying to the user. We do the macro-ish syntax for deref pats, since users won't be writing builtin # deref themselves.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, lol, I see. It's due to #124619.

In that case, I wonder if we need to fix deref patterns too here...

Copy link
Member Author

Choose a reason for hiding this comment

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

I cc'ed #124619 to give some extra context but it's not really related. #124619 describes a rustfmt bug which I plan on fixing out of tree / in the rustfmt repo.

Copy link
Member

Choose a reason for hiding this comment

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

oh, in that case, I feel like we might want to keep this as type_ascribe!() lol -- but whatever, if you don't agree, i don't particularly care

Copy link
Member Author

@fmease fmease May 3, 2024

Choose a reason for hiding this comment

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

I'm definitely not a fan of builtin #'s visual appearance, it looks grotesque and verbose. However, printing a fake macro call I like even less.

Copy link
Member

Choose a reason for hiding this comment

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

I think it better to print the builtin

Copy link
Member

Choose a reason for hiding this comment

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

because that doesn't need to be imported

self.popen();
self.ibox(0);
self.print_expr(expr, FixupContext::default());

Expand All @@ -431,7 +432,7 @@ impl<'a> State<'a> {
self.print_type(ty);

self.end();
self.word(")");
self.pclose();
}
ast::ExprKind::Let(pat, scrutinee, _, _) => {
self.print_let(pat, scrutinee, fixup);
Expand Down Expand Up @@ -657,15 +658,15 @@ impl<'a> State<'a> {
);
}
ast::ExprKind::InlineAsm(a) => {
// FIXME: This should have its own syntax, distinct from a macro invocation.
// FIXME: Print `builtin # asm` once macro `asm` uses `builtin_syntax`.
self.word("asm!");
self.print_inline_asm(a);
}
ast::ExprKind::FormatArgs(fmt) => {
// FIXME: This should have its own syntax, distinct from a macro invocation.
// FIXME: Print `builtin # format_args` once macro `format_args` uses `builtin_syntax`.
self.word("format_args!");
self.popen();
self.rbox(0, Inconsistent);
self.ibox(0);
self.word(reconstruct_format_args_template_string(&fmt.template));
for arg in fmt.arguments.all_args() {
self.word_space(",");
Expand All @@ -677,7 +678,7 @@ impl<'a> State<'a> {
ast::ExprKind::OffsetOf(container, fields) => {
self.word("builtin # offset_of");
self.popen();
self.rbox(0, Inconsistent);
self.ibox(0);
self.print_type(container);
self.word(",");
self.space();
Expand All @@ -690,8 +691,8 @@ impl<'a> State<'a> {
self.print_ident(field);
}
}
self.pclose();
self.end();
self.pclose();
}
ast::ExprKind::MacCall(m) => self.print_mac(m),
ast::ExprKind::Paren(e) => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_pretty/src/pprust/state/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ impl<'a> State<'a> {
self.bclose(item.span, empty);
}
ast::ItemKind::GlobalAsm(asm) => {
// FIXME: Print `builtin # global_asm` once macro `global_asm` uses `builtin_syntax`.
self.head(visibility_qualified(&item.vis, "global_asm!"));
self.print_inline_asm(asm);
self.word(";");
Expand Down
Loading