Skip to content

Conversation

ada4a
Copy link
Contributor

@ada4a ada4a commented Sep 10, 2025

Fixes #13883

changelog: [manual_assert]: don't lint on if cfg!(..) { panic!(..) }

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 10, 2025

r? @Alexendoo

rustbot has assigned @Alexendoo.
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

Copy link
Member

@samueltardieu samueltardieu left a comment

Choose a reason for hiding this comment

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

Is that really a common idiom? If this is used through the debug_panic crate for example, shouldn't we instead refrain from linting code expanded from an external macro when we detect this pattern?

r? samueltardieu

View changes since this review

@rustbot rustbot assigned samueltardieu and unassigned Alexendoo Sep 14, 2025
@ada4a
Copy link
Contributor Author

ada4a commented Sep 14, 2025

Is that really a common idiom?

That is what the issue author was suggesting at least.

If this is used through the debug_panic crate for example, shouldn't we instead refrain from linting code expanded from an external macro when we detect this pattern?

I think that's an orthogonal but good idea, yes.

@samueltardieu
Copy link
Member

Is that really a common idiom?

That is what the issue author was suggesting at least.

Yes, but it would be better if it was backed up by some numbers. Otherwise, we are just preventing the lint from triggering in legitimate situations. If this is not that common, the lint can just be allowed by users who want to do this.

@ada4a
Copy link
Contributor Author

ada4a commented Sep 16, 2025

In that case, would you want to go ask the issue author for some numbers/close the issue , while I implement the from-expansion check?

EDIT: there's a from-expansion check already in place:

&& !expr.span.from_expansion()

@samueltardieu
Copy link
Member

@oriongonza ?

@rustbot
Copy link
Collaborator

rustbot commented Oct 8, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

manual_assert triggers for cfg!
4 participants