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.

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

Jump to bottom

Don't bail out early when checking invalid repr attr #111062

Merged
merged 1 commit into from
May 3, 2023
Merged

Don't bail out early when checking invalid repr attr #111062

merged 1 commit into from
May 3, 2023

Conversation

clubby789
Copy link
Contributor

Fixes #111051

An invalid repr delays a bug. If there are other invalid attributes on the item, we emit a warning and exit without re-checking the repr here, so no error is emitted and the delayed bug ICEs

@rustbot
Copy link
Collaborator

rustbot commented May 1, 2023

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

@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 1, 2023
@CAD97
Copy link
Contributor

CAD97 commented May 2, 2023

Imho it seems more that the issue is that doc(invalid) is being reported as an invalid attribute (thus short circuiting here) even though it's only a warning, rather than the short circuit being wrong, per-say. (That is, it should be in the lint-only checks while it's not a hard error.) On the other hand, though, it seems reasonable to not short circuit and show all of the invalid builtin attribute errors; one being invalid shouldn't have any impact on if another is.

@petrochenkov
Copy link
Contributor

On the other hand, though, it seems reasonable to not short circuit and show all of the invalid builtin attribute errors; one being invalid shouldn't have any impact on if another is.

I think this will be both simpler and more useful.
@bors r+

@bors
Copy link
Contributor

bors commented May 3, 2023

📌 Commit 0453cda has been approved by petrochenkov

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-review Status: Awaiting review from the assignee but also interested parties. labels May 3, 2023
@Dylan-DPC Dylan-DPC mentioned this pull request May 3, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2023
@bors
Auto merge of rust-lang#111153 - Dylan-DPC:rollup-0pq0hh3, r=Dylan-DPC
473f916 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#107978 (Correctly convert an NT path to a Win32 path in `read_link`)
 - rust-lang#110436 (Support loading version information from xz tarballs)
 - rust-lang#110791 (Implement negative bounds for internal testing purposes)
 - rust-lang#110874 (Adjust obligation cause code for `find_and_report_unsatisfied_index_impl`)
 - rust-lang#110908 (resolve: One more attempt to simplify `module_children`)
 - rust-lang#110943 (interpret: fail more gracefully on uninit unsized locals)
 - rust-lang#111062 (Don't bail out early when checking invalid `repr` attr)
 - rust-lang#111069 (remove pointless `FIXME` in `bootstrap::download`)
 - rust-lang#111086 (Remove `MemEncoder`)
 - rust-lang#111097 (Avoid ICEing miri on layout query cycles)
 - rust-lang#111112 (Add some triagebot notifications for nnethercote.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fce0741 into rust-lang:master May 3, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbstj fbstj fbstj left review comments

Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.

ICE: unrecognized representation hint if repr == doc attr
6 participants
@clubby789 @rustbot @CAD97 @petrochenkov @bors @fbstj