Skip to content

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Mar 12, 2023

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Mar 12, 2023

r? @jackh726

(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 Mar 12, 2023
@jackh726
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 13, 2023

📌 Commit b16d6cc has been approved by jackh726

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 Mar 13, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 13, 2023
@@ -1999,6 +1999,9 @@ impl BorrowKind {
}

impl BinOp {
/// The checkable operators are those whose overflow checking behavior is controlled by
/// -Coverflow-checks option. The remaining operators have either no overflow conditions (e.g.,
/// BitAnd, BitOr, BitXor) or are always checked for overflow (e.g., Div, Rem).
Copy link
Member

Choose a reason for hiding this comment

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

UnOp does not have is_checkable. Given these docs, that seems surprising? Neg behaves like the 'checkable' bin ops, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UnOp::Neg would be checkable. What do you find surprising about that in context of this documentation?

Copy link
Member

Choose a reason for hiding this comment

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

The surprising part is that there is no such function on UnOp -- that indicates some asymmetry in how UnOp vs BinOp are handled.

Should we also have this on UnOp, and then replace the existing direct matching on Neg that I assume is happening with an is_checkable call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Negation has a dedicated assertion kind for it, so there is no need for UnOp::is_checkable (see git grep -C20 is_checkable).

Copy link
Member

Choose a reason for hiding this comment

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

What does this have to do with assertion kinds? I was thinking of MIR building.

Though with #108282, MIR building does not use this function any more anyway. Then it's only the codegen backends and yeah there it has to do with assertion kinds.

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2023
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#108419 (Stabilize `atomic_as_ptr`)
 - rust-lang#108507 (use `as_ptr` to determine the address of atomics)
 - rust-lang#108607 (Don't use fd-lock on Solaris in bootstrap)
 - rust-lang#108830 (Treat projections with infer as placeholder during fast reject in new solver)
 - rust-lang#109055 (create `config::tests::detect_src_and_out` test for bootstrap)
 - rust-lang#109058 (Document BinOp::is_checkable)
 - rust-lang#109081 (simd-wide-sum test: adapt for LLVM 17 codegen change)
 - rust-lang#109083 (Update books)
 - rust-lang#109088 (Gracefully handle `#[target_feature]` on statics)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9fc8de0 into rust-lang:master Mar 14, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 14, 2023
@tmiasko tmiasko deleted the is-checkable branch March 14, 2023 13:24
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.

5 participants