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

Rename ConstPropLint to KnownPanicsLint #121286

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

gurry
Copy link
Contributor

@gurry gurry commented Feb 19, 2024

OverflowLint is a clearer name because it communicates what the lint does instead of the underlying mechanism it uses (const propagation) which should be of secondary concern.

OverflowLint isn't the most accurate name because the lint looks for other errors as well such as division by zero not just overflows, but I couldn't think of another equally succinct name.

As a part of this change. I've also added/updated some of the comments.

cc @RalfJung @oli-obk for visibility in case you go looking for the lint using the old name.

Edit:

Changed the name from OverflowLint to KnownPanicsLint

@rustbot
Copy link
Collaborator

rustbot commented Feb 19, 2024

r? @davidtwco

rustbot has assigned @davidtwco.
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 Feb 19, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 19, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt


// FIXME(oli-obk, eddyb) Optimize locals (or even local paths) to hold
Copy link
Contributor Author

@gurry gurry Feb 19, 2024

Choose a reason for hiding this comment

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

Removed this comment as it talks about "optimizing" locals while the code does not optimize anything any more. The comment is a relic from the glorious past when it used to be a full-blooded const propagator until it was brutally gutted and turned into a mere lint.

@Noratrieb
Copy link
Member

this lint is about detecting code patterns that are known to cause panics at runtime. maybe KnownPanic? KnownRuntimePanic? UnconditionalPanic?

@gurry
Copy link
Contributor Author

gurry commented Feb 19, 2024

Adding one more to the mix, ConstOpsLint since it checks that operations on constants do not panic.

Let's hear from others. I'll change it to whatever we all agree on.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 19, 2024

KnownRuntimePanic

This seems best. We can't prove that a panic is actually unconditional 😅

@gurry
Copy link
Contributor Author

gurry commented Feb 19, 2024

How about just KnownPanicsLint instead of KnownRuntimePanicsLint for brevity?

(Note that I'm also suggesting using the plural "Panics" instead of "Panic")

@gurry
Copy link
Contributor Author

gurry commented Feb 19, 2024

Or just PanicsLint or even RuntimePanicsLint 'cause "Known" does not seem to add much value.

@gurry
Copy link
Contributor Author

gurry commented Feb 19, 2024

Or PanickingOpsLint (sorry I'm getting a bit carried away here).

@Noratrieb
Copy link
Member

I think it adds a lot of value. PanicsLint just implies a lint that finds potential panics in the program, but KnownPanicsLint makes it clear that these are places we know will panic.

@gurry
Copy link
Contributor Author

gurry commented Feb 19, 2024

Makes sense. Let's go with KnownPanicsLint.

Will wait for a day. If no one objects, will make the change tomorrow.

It is a clearer name because it communicates what the lint does
instead of the underlying mechanism it uses (const propagation).
@gurry gurry changed the title Rename ConstPropLint to OverflowLint Rename ConstPropLint to KnownPanicsLint Feb 20, 2024
@gurry
Copy link
Contributor Author

gurry commented Feb 20, 2024

Renamed to KnownPanicsLint and ready for review

@oli-obk
Copy link
Contributor

oli-obk commented Feb 20, 2024

r? @oli-obk

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 20, 2024

📌 Commit 42c4df0 has been approved by oli-obk

It is now in the queue for this repository.

@rustbot rustbot assigned oli-obk and unassigned davidtwco Feb 20, 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 Feb 20, 2024
Noratrieb added a commit to Noratrieb/rust that referenced this pull request Feb 20, 2024
…-obk

Rename `ConstPropLint` to `KnownPanicsLint`

`OverflowLint` is a clearer name because it communicates what the lint does instead of the underlying mechanism it uses (const propagation) which should be of secondary concern.

`OverflowLint` isn't the most accurate name because the lint looks for other errors as well such as division by zero not just overflows, but I couldn't think of another equally succinct name.

As a part of this change. I've also added/updated some of the comments.

cc `@RalfJung` `@oli-obk` for visibility in case you go looking for the lint using the old name.

Edit:

Changed the name from `OverflowLint` to `KnownPanicsLint`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 20, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#121167 (resolve: Scale back unloading of speculatively loaded crates)
 - rust-lang#121196 (Always inline check in `assert_unsafe_precondition` with cfg(debug_assertions))
 - rust-lang#121206 (Top level error handling)
 - rust-lang#121223 (intrinsics::simd: add missing functions)
 - rust-lang#121241 (Implement `NonZero` traits generically.)
 - rust-lang#121242 (Generate `getelementptr` instead of `inttoptr` for `ptr::invalid`)
 - rust-lang#121278 (Remove the "codegen" profile from bootstrap)
 - rust-lang#121286 (Rename `ConstPropLint` to `KnownPanicsLint`)

r? `@ghost`
`@rustbot` modify labels: rollup
Noratrieb added a commit to Noratrieb/rust that referenced this pull request Feb 20, 2024
…-obk

Rename `ConstPropLint` to `KnownPanicsLint`

`OverflowLint` is a clearer name because it communicates what the lint does instead of the underlying mechanism it uses (const propagation) which should be of secondary concern.

`OverflowLint` isn't the most accurate name because the lint looks for other errors as well such as division by zero not just overflows, but I couldn't think of another equally succinct name.

As a part of this change. I've also added/updated some of the comments.

cc ``@RalfJung`` ``@oli-obk`` for visibility in case you go looking for the lint using the old name.

Edit:

Changed the name from `OverflowLint` to `KnownPanicsLint`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 20, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#121167 (resolve: Scale back unloading of speculatively loaded crates)
 - rust-lang#121196 (Always inline check in `assert_unsafe_precondition` with cfg(debug_assertions))
 - rust-lang#121241 (Implement `NonZero` traits generically.)
 - rust-lang#121278 (Remove the "codegen" profile from bootstrap)
 - rust-lang#121286 (Rename `ConstPropLint` to `KnownPanicsLint`)
 - rust-lang#121291 (target: Revert default to the medium code model on LoongArch targets)
 - rust-lang#121302 (Remove `RefMutL` hack in `proc_macro::bridge`)
 - rust-lang#121318 (Trigger `unsafe_code` lint on invocations of `global_asm`)

Failed merges:

 - rust-lang#121206 (Top level error handling)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bc7489c into rust-lang:master Feb 20, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 20, 2024
Rollup merge of rust-lang#121286 - gurry:constprop-lint-rename, r=oli-obk

Rename `ConstPropLint` to `KnownPanicsLint`

`OverflowLint` is a clearer name because it communicates what the lint does instead of the underlying mechanism it uses (const propagation) which should be of secondary concern.

`OverflowLint` isn't the most accurate name because the lint looks for other errors as well such as division by zero not just overflows, but I couldn't think of another equally succinct name.

As a part of this change. I've also added/updated some of the comments.

cc ```@RalfJung``` ```@oli-obk``` for visibility in case you go looking for the lint using the old name.

Edit:

Changed the name from `OverflowLint` to `KnownPanicsLint`
@gurry gurry deleted the constprop-lint-rename branch February 21, 2024 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

6 participants