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

Conversation

fmease
Copy link
Member

@fmease fmease commented May 2, 2024

Follow-up to #122806.
CC rust-lang/rustfmt#6159.

@fmease fmease added the A-pretty Area: Pretty printing. label May 2, 2024
@rustbot
Copy link
Collaborator

rustbot commented May 2, 2024

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 2, 2024
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

whatever tho @ my last comment

@@ -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

@compiler-errors
Copy link
Member

r? compiler-errors @bors r+ rollup

@bors
Copy link
Contributor

bors commented May 3, 2024

📌 Commit 3a3df3e has been approved by compiler-errors

It is now in the queue for this repository.

@rustbot rustbot assigned compiler-errors and unassigned oli-obk May 3, 2024
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 3, 2024
fmease added a commit to fmease/rust that referenced this pull request May 3, 2024
…yn, r=compiler-errors

AST pretty: Use `builtin_syntax` for type ascription

Follow-up to rust-lang#122806.
CC #124619.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#124441 (String.truncate comment microfix (greater or equal))
 - rust-lang#124594 (run-make-support: preserve tooks.mk behavior for EXTRACXXFLAGS)
 - rust-lang#124604 (library/std: Remove unused `gimli-symbolize` feature)
 - rust-lang#124607 (`rustc_expand` cleanups)
 - rust-lang#124609 (variable-precision float operations can differ depending on optimization levels)
 - rust-lang#124610 (Tweak `consts_may_unify`)
 - rust-lang#124637 (AST pretty: Use `builtin_syntax` for type ascription)

Failed merges:

 - rust-lang#124638 (Move some tests from `rustc_expand` to `rustc_parse`.)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#123480 (deref patterns: impl `DerefPure` for more std types)
 - rust-lang#124412 (io safety: update Unix explanation to use `Arc`)
 - rust-lang#124441 (String.truncate comment microfix (greater or equal))
 - rust-lang#124594 (run-make-support: preserve tooks.mk behavior for EXTRACXXFLAGS)
 - rust-lang#124604 (library/std: Remove unused `gimli-symbolize` feature)
 - rust-lang#124607 (`rustc_expand` cleanups)
 - rust-lang#124609 (variable-precision float operations can differ depending on optimization levels)
 - rust-lang#124610 (Tweak `consts_may_unify`)
 - rust-lang#124626 (const_eval_select: add tracking issue)
 - rust-lang#124637 (AST pretty: Use `builtin_syntax` for type ascription)

Failed merges:

 - rust-lang#124638 (Move some tests from `rustc_expand` to `rustc_parse`.)

r? `@ghost`
`@rustbot` modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
Rollup merge of rust-lang#124637 - fmease:ast-pretty-ty-asc-builtin-syn, r=compiler-errors

AST pretty: Use `builtin_syntax` for type ascription

Follow-up to rust-lang#122806.
CC #124619.
@bors bors merged commit e6c82d9 into rust-lang:master May 3, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 3, 2024
@fmease fmease deleted the ast-pretty-ty-asc-builtin-syn branch May 3, 2024 13:34
@RalfJung
Copy link
Member

RalfJung commented May 3, 2024

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pretty Area: Pretty printing. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants