Skip to content

Conversation

Zalathar
Copy link
Contributor

This should be nicer and more type-safe than the old typedef for c_int/i32.

Using #[repr(transparent)] should ensure that it's still ABI-compatible.

@rustbot
Copy link
Collaborator

rustbot commented Aug 24, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. F-autodiff `#![feature(autodiff)]` 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 Aug 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 24, 2025

Some changes occurred in compiler/rustc_codegen_llvm/src/builder/autodiff.rs

cc @ZuseZ4

Copy link
Member

@hkBst hkBst left a comment

Choose a reason for hiding this comment

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

Looks good, except please clarify the behavior of non-standard values (those greater than 1, and smaller than 0).

View changes since this review

@Zalathar
Copy link
Contributor Author

Added a comment explaining the handling of boolean values that aren't 0 or 1 (diff).

Note that technically this is a change from the pre-existing behaviour (which tends to compare against True), but it's unlikely that this matters, and if it does matter then the new behaviour seems more likely to be correct.

@workingjubilee
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 24, 2025

📌 Commit b4e97e5 has been approved by workingjubilee

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 Aug 24, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 24, 2025
cg_llvm: Replace the `llvm::Bool` typedef with a proper newtype

This should be nicer and more type-safe than the old typedef for `c_int`/`i32`.

Using `#[repr(transparent)]` should ensure that it's still ABI-compatible.
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 24, 2025
cg_llvm: Replace the `llvm::Bool` typedef with a proper newtype

This should be nicer and more type-safe than the old typedef for `c_int`/`i32`.

Using `#[repr(transparent)]` should ensure that it's still ABI-compatible.
bors added a commit that referenced this pull request Aug 24, 2025
Rollup of 6 pull requests

Successful merges:

 - #135761 (Dial down detail of B-tree description)
 - #144373 (remove deprecated Error::description in impls)
 - #145620 (Account for impossible bounds making seemingly unsatisfyable dyn-to-dyn casts)
 - #145783 (add span to struct pattern rest (..))
 - #145817 (cg_llvm: Replace the `llvm::Bool` typedef with a proper newtype)
 - #145820 (raw-dylib-elf: set correct `DT_VERDEFNUM`)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 24, 2025
cg_llvm: Replace the `llvm::Bool` typedef with a proper newtype

This should be nicer and more type-safe than the old typedef for `c_int`/`i32`.

Using `#[repr(transparent)]` should ensure that it's still ABI-compatible.
bors added a commit that referenced this pull request Aug 25, 2025
Rollup of 5 pull requests

Successful merges:

 - #135761 (Dial down detail of B-tree description)
 - #144373 (remove deprecated Error::description in impls)
 - #145620 (Account for impossible bounds making seemingly unsatisfyable dyn-to-dyn casts)
 - #145817 (cg_llvm: Replace the `llvm::Bool` typedef with a proper newtype)
 - #145820 (raw-dylib-elf: set correct `DT_VERDEFNUM`)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 25, 2025
cg_llvm: Replace the `llvm::Bool` typedef with a proper newtype

This should be nicer and more type-safe than the old typedef for `c_int`/`i32`.

Using `#[repr(transparent)]` should ensure that it's still ABI-compatible.
@Zalathar
Copy link
Contributor Author

Tracking down a failure in rollup #145833.

@bors try jobs=dist-various-2

rust-bors bot added a commit that referenced this pull request Aug 25, 2025
cg_llvm: Replace the `llvm::Bool` typedef with a proper newtype

try-job: dist-various-2
@rust-bors

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Aug 25, 2025

☀️ Try build successful (CI)
Build commit: af6008d (af6008d253b2b2b8b0a567bbff889f1d42299547, parent: a1dbb443527bd126452875eb5d5860c1d001d761)

bors added a commit that referenced this pull request Aug 25, 2025
Rollup of 10 pull requests

Successful merges:

 - #135761 (Dial down detail of B-tree description)
 - #145620 (Account for impossible bounds making seemingly unsatisfyable dyn-to-dyn casts)
 - #145788 (Fix attribute target checking for macro calls)
 - #145794 (bootstrap.py: Improve CPU detection on NetBSD)
 - #145817 (cg_llvm: Replace the `llvm::Bool` typedef with a proper newtype)
 - #145820 (raw-dylib-elf: set correct `DT_VERDEFNUM`)
 - #145828 (Update `bitflags` to 2.9.3.)
 - #145830 (Remove the lifetime from `ExpTokenPair`/`SeqSep`.)
 - #145836 (Remove outdated bug comments)
 - #145842 (rustc-dev-guide subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0b67285 into rust-lang:master Aug 25, 2025
11 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 25, 2025
rust-timer added a commit that referenced this pull request Aug 25, 2025
Rollup merge of #145817 - Zalathar:llvm-bool, r=workingjubilee

cg_llvm: Replace the `llvm::Bool` typedef with a proper newtype

This should be nicer and more type-safe than the old typedef for `c_int`/`i32`.

Using `#[repr(transparent)]` should ensure that it's still ABI-compatible.
@Zalathar Zalathar deleted the llvm-bool branch August 25, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. F-autodiff `#![feature(autodiff)]` 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