Skip to content

Improve -Z crate-attr diagnostics #138336

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 14, 2025
Merged

Improve -Z crate-attr diagnostics #138336

merged 1 commit into from
Apr 14, 2025

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 11, 2025 •
edited
Loading

  • Show the #![ ... ] in the span (to make it clear that it should not
    be included in the CLI argument)
  • Show more detailed errors when the crate has valid token trees but
    invalid syntax.
    Previously, crate-attr=feature(foo),feature(bar) would just say
    "invalid crate attribute" and point at the comma. Now, it explicitly
    says that the comma was unexpected, which is useful when using
    --error-format=short. It also fixes the column to show the correct
    span.
  • Recover from parse errors. Previously we would abort immediately on
    syntax errors; now we go on to try and type-check the rest of the
    crate.

The new diagnostic code also happens to be slightly shorter.

r? diagnostics

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 11, 2025
@jyn514 jyn514 force-pushed the crate-attr-diagnostics branch from 5e4599d to 2d05a27 Compare March 11, 2025 04:39
@rustbot rustbot removed the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Mar 11, 2025
@jyn514 jyn514 changed the title Crate attr diagnostics Improve -Z crate-attr diagnostics Mar 11, 2025
@jyn514 jyn514 force-pushed the crate-attr-diagnostics branch 2 times, most recently from 7fc52b8 to 74e6e2d Compare March 11, 2025 04:42
@jyn514 jyn514 mentioned this pull request Mar 11, 2025
Open
6 tasks
@rust-log-analyzer

This comment has been minimized.

# to view
@jyn514 jyn514 force-pushed the crate-attr-diagnostics branch from 74e6e2d to 4b25b06 Compare March 11, 2025 12:22
@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 11, 2025 •
edited
Loading

Move crate-attr parsing from rustc_builtin_macros to
rustc_parse. This is not a built-in macro.

All kind of code injected by rustc consistently lives in rustc_builtin_macros - prelude injection, test harness, etc (search for ::inject(), I suggest keeping it in the current location.
(The logic is that attribute/prelude/etc injection is not used to parse anything, so it's not in rustc_parse.)

@rust-log-analyzer

This comment has been minimized.

# to view
bjorn3
bjorn3 reviewed Mar 11, 2025
View reviewed changes
@jyn514 jyn514 force-pushed the crate-attr-diagnostics branch from 4b25b06 to 86072ff Compare March 12, 2025 02:31
@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Mar 12, 2025
@jyn514
Copy link
Member Author

jyn514 commented Mar 12, 2025

All kind of code injected by rustc consistently lives in rustc_builtin_macros - prelude injection, test harness, etc (search for ::inject(), I suggest keeping it in the current location.
(The logic is that attribute/prelude/etc injection is not used to parse anything, so it's not in rustc_parse.)

ok, done. fortunately parse_attribute was public (i guess another motivation of not having this in rustc_parse is so that we can't use parser internals).

@jyn514 jyn514 force-pushed the crate-attr-diagnostics branch from 86072ff to 42b890b Compare March 12, 2025 02:32
jyn514
jyn514 commented Mar 12, 2025
View reviewed changes
@jyn514 jyn514 force-pushed the crate-attr-diagnostics branch from 42b890b to 4a301c7 Compare March 12, 2025 02:33
@jieyouxu jieyouxu added the -Zcrate-attr Unstable option: -Zcrate-attr label Mar 16, 2025
@jyn514 jyn514 mentioned this pull request Mar 17, 2025
Open
@jyn514
Copy link
Member Author

jyn514 commented Mar 26, 2025

👋 @estebank do you know when you'll get a chance to look at this? would you like me to assign a different reviewer?

@bors
Copy link
Collaborator

bors commented Apr 3, 2025

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

@jyn514
Copy link
Member Author

jyn514 commented Apr 3, 2025

r? diagnostics

@rustbot rustbot assigned compiler-errors and unassigned estebank Apr 3, 2025
@compiler-errors
Copy link
Member

Needs a rebase, but otherwise r=me

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 13, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@jyn514 jyn514 force-pushed the crate-attr-diagnostics branch from 4a301c7 to 18cdd59 Compare April 13, 2025 20:27
@rust-log-analyzer

This comment has been minimized.

# to view
@jyn514
Improve -Z crate-attr diagnostics
d50a8d5 
- Show the `#![ ... ]` in the span (to make it clear that it should not
  be included in the CLI argument)
- Show more detailed errors when the crate has valid token trees but
  invalid syntax.
  Previously, `crate-attr=feature(foo),feature(bar)` would just say
  "invalid crate attribute" and point at the comma. Now, it explicitly
  says that the comma was unexpected, which is useful when using
  `--error-format=short`. It also fixes the column to show the correct
  span.
- Recover from parse errors. Previously we would abort immediately on
  syntax errors; now we go on to try and type-check the rest of the
  crate.

The new diagnostic code also happens to be slightly shorter.
@jyn514 jyn514 force-pushed the crate-attr-diagnostics branch from 18cdd59 to d50a8d5 Compare April 13, 2025 20:46
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 14, 2025

📌 Commit d50a8d5 has been approved by compiler-errors

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 14, 2025
@jhpratt jhpratt mentioned this pull request Apr 14, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2025
@bors
Auto merge of rust-lang#139781 - jhpratt:rollup-qadsjvb, r=jhpratt
5961e5b 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#138336 (Improve `-Z crate-attr` diagnostics)
 - rust-lang#139636 (Encode dep node edge count as u32 instead of usize)
 - rust-lang#139666 (cleanup `mir_borrowck`)
 - rust-lang#139695 (compiletest: consistently use `camino::{Utf8Path,Utf8PathBuf}` throughout)
 - rust-lang#139699 (Proactively update coroutine drop shim's phase to account for later passes applied during shim query)
 - rust-lang#139718 (enforce unsafe attributes in pre-2024 editions by default)
 - rust-lang#139722 (Move some things to rustc_type_ir)
 - rust-lang#139760 (UI tests: migrate remaining compile time `error-pattern`s to line annotations when possible)
 - rust-lang#139776 (Switch attrs to `diagnostic::on_unimplemented`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2b6835b into rust-lang:master Apr 14, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2025
@rust-timer
Unrolled build for rust-lang#138336
a6d5564 
Rollup merge of rust-lang#138336 - jyn514:crate-attr-diagnostics, r=compiler-errors

Improve `-Z crate-attr` diagnostics

- Show the `#![ ... ]` in the span (to make it clear that it should not
  be included in the CLI argument)
- Show more detailed errors when the crate has valid token trees but
  invalid syntax.
  Previously, `crate-attr=feature(foo),feature(bar)` would just say
  "invalid crate attribute" and point at the comma. Now, it explicitly
  says that the comma was unexpected, which is useful when using
  `--error-format=short`. It also fixes the column to show the correct
  span.
- Recover from parse errors. Previously we would abort immediately on
  syntax errors; now we go on to try and type-check the rest of the
  crate.

The new diagnostic code also happens to be slightly shorter.

r? diagnostics
@jyn514 jyn514 deleted the crate-attr-diagnostics branch April 15, 2025 13:39
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
-Zcrate-attr Unstable option: -Zcrate-attr S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.88.0
Development

Successfully merging this pull request may close these issues.
9 participants
@jyn514 @rust-log-analyzer @petrochenkov @bors @compiler-errors @rustbot @bjorn3 @estebank @jieyouxu