Skip to content

Don't silently ignore errors that happen during rendering#104995

Closed
jyn514 wants to merge 1 commit intorust-lang:masterfrom
jyn514:lint-errors
Closed

Don't silently ignore errors that happen during rendering#104995
jyn514 wants to merge 1 commit intorust-lang:masterfrom
jyn514:lint-errors

Conversation

@jyn514
Copy link
Member

@jyn514 jyn514 commented Nov 27, 2022

This caught a real, existing bug.

Found in #104994

@rustbot
Copy link
Collaborator

rustbot commented Nov 27, 2022

r? @CraftSpider

(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-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 27, 2022
@jyn514 jyn514 added the relnotes Marks issues that should be documented in the release notes of the next release. label Nov 27, 2022
@jyn514
Copy link
Member Author

jyn514 commented Nov 27, 2022

Adding relnotes since this will probably break CI for someone; I don't think we need to run crater since this is an obviously correct fix and it will get run for beta anyway.

@Manishearth
Copy link
Member

Manishearth commented Nov 27, 2022

Oof, good catch

@bors r+

(Unsure if this ought to be marked as rollup=always)

@bors
Copy link
Collaborator

bors commented Nov 27, 2022

📌 Commit 6a25bfa has been approved by Manishearth

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 Nov 27, 2022
@jyn514
Copy link
Member Author

jyn514 commented Nov 28, 2022

@workingjubilee points out we should likely do the same thing for wrap_return.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 29, 2022
@matthiaskrgr
Rollup merge of rust-lang#104995 - jyn514:lint-errors, r=Manishearth
8937a2e 
Don't silently ignore errors that happen during rendering

This caught a real, existing bug.

Found in rust-lang#104994
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 29, 2022
Closed
@matthiaskrgr
Copy link
Member

matthiaskrgr commented Nov 29, 2022

@bors r- failed in a rollup
https://github.com/rust-lang-ci/rust/actions/runs/3577882960/jobs/6017395838#step:26:39356
#105059

@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 Nov 29, 2022
riking
riking reviewed Dec 6, 2022
View reviewed changes
src/test/rustdoc-ui/const-evalutation-ice.rs
@@ -8,3 +8,4 @@ pub struct S {
}
Copy link

@riking riking Dec 6, 2022

Choose a reason for hiding this comment

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

update test's file comment

@CraftSpider CraftSpider removed their assignment Dec 6, 2022
@CraftSpider
Copy link
Contributor

CraftSpider commented Dec 6, 2022

Not sure I can still do this, but:
r? @rust-lang/rustdoc

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Dec 6, 2022

You can and thanks for it! We'll try to take a look soon.

@GuillaumeGomez GuillaumeGomez mentioned this pull request Dec 7, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2022
@matthiaskrgr
Rollup merge of rust-lang#105427 - GuillaumeGomez:dont-silently-ignor…
b475163 
…e-rustdoc-errors, r=notriddle

Dont silently ignore rustdoc errors

I applied the suggestions from rust-lang#104995 and also checked the rustdoc-ui error but couldn't reproduce it.

r? `@notriddle`
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Dec 8, 2022

I have added the comment you mentioned into #105427. The CI failed though on linux i686, not sure why. I ignored this platform in the test and just started a try build to see if this specific to this platform or not. Please feel free to close my PR if you intend to update yours.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 9, 2022
@matthiaskrgr
Rollup merge of rust-lang#105427 - GuillaumeGomez:dont-silently-ignor…
4b09861 
…e-rustdoc-errors, r=notriddle

Dont silently ignore rustdoc errors

I applied the suggestions from rust-lang#104995 and also checked the rustdoc-ui error but couldn't reproduce it.

r? `@notriddle`
@bors
Copy link
Collaborator

bors commented Dec 9, 2022

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

@jyn514 jyn514 closed this Dec 9, 2022
@jyn514 jyn514 deleted the lint-errors branch December 9, 2022 13:58
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Dec 11, 2022
@matthiaskrgr
Rollup merge of #105427 - GuillaumeGomez:dont-silently-ignore-rustdoc…
7811ff8 
…-errors, r=notriddle

Dont silently ignore rustdoc errors

I applied the suggestions from rust-lang/rust#104995 and also checked the rustdoc-ui error but couldn't reproduce it.

r? `@notriddle`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@riking riking riking left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@notriddle notriddle

Labels

relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@jyn514 @rustbot @Manishearth @bors @matthiaskrgr @CraftSpider @GuillaumeGomez @riking @notriddle