-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Prevent ICE in autodiff validation by emitting user-friendly errors #138231
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
This comment has been minimized.
This comment has been minimized.
builtin_macros_autodiff_ty_activity = {$act} can not be used for this type | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why I had builtin_macros_autodiff_unknown_activity
not grouped together with the rest, but maybe combine them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just grouped them so all autodiff-related errors are together.
@rustbot label +F-autodiff |
Some changes occurred in compiler/rustc_codegen_ssa/src/codegen_attrs.rs Some changes occurred in compiler/rustc_codegen_ssa |
@bors delegate=ZuseZ4 |
Contrary to what the bot said, feel free to |
@Sa4dUs It's not strictly necessary, but if you have some time, could you please remove the Remove test_ad commit and just update the previous commit to not include the binary blob? When possible it's preferable to have a cleaner history and let every commit build on it's own, since bors will just apply all of your commits unchanged (and not squash them). |
Done! Sorry for the oversight. |
Thanks! @bors r+ |
@bors rollup |
Prevent ICE in autodiff validation by emitting user-friendly errors This PR moves `valid_ret_activity` and `valid_input_activity` checks to the macro expansion phase in compiler/rustc_builtin_macros/src/autodiff.rs, replacing the following internal compiler error (ICE): ``` error: internal compiler error: compiler/rustc_codegen_ssa/src/codegen_attrs.rs:935:13: Invalid input activity Dual for Reverse mode ``` with a more user-friendly message. The issue specifically affected the test file `tests/ui/autodiff/autodiff_illegal.rs`, impacting the functions `f5` and `f6`. The ICE can be reproduced by following [Enzyme's Rustbook](https://enzymead.github.io/rustbook/installation.html) installation guide. Additionally, this PR adds tests for invalid return activity in `autodiff_illegal.rs`, which previously triggered an unnoticed ICE before these fixes. r? `@oli-obk`
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#137715 (Allow int literals for pattern types with int base types) - rust-lang#138002 (Disable CFI for weakly linked syscalls) - rust-lang#138051 (Add support for downloading GCC from CI) - rust-lang#138231 (Prevent ICE in autodiff validation by emitting user-friendly errors) - rust-lang#138245 (stabilize `ci_rustc_if_unchanged_logic` test for local environments) - rust-lang#138256 (Do not feed anon const a type that references generics that it does not have) - rust-lang#138284 (Do not write user type annotation for const param value path) - rust-lang#138296 (Remove `AdtFlags::IS_ANONYMOUS` and `Copy`/`Clone` condition for anonymous ADT) - rust-lang#138352 (miri native_calls: ensure we actually expose *mutable* provenance to the memory FFI can access) - rust-lang#138354 (remove redundant `body` arguments) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#137715 (Allow int literals for pattern types with int base types) - rust-lang#138002 (Disable CFI for weakly linked syscalls) - rust-lang#138051 (Add support for downloading GCC from CI) - rust-lang#138231 (Prevent ICE in autodiff validation by emitting user-friendly errors) - rust-lang#138245 (stabilize `ci_rustc_if_unchanged_logic` test for local environments) - rust-lang#138256 (Do not feed anon const a type that references generics that it does not have) - rust-lang#138284 (Do not write user type annotation for const param value path) - rust-lang#138296 (Remove `AdtFlags::IS_ANONYMOUS` and `Copy`/`Clone` condition for anonymous ADT) - rust-lang#138352 (miri native_calls: ensure we actually expose *mutable* provenance to the memory FFI can access) - rust-lang#138354 (remove redundant `body` arguments) r? `@ghost` `@rustbot` modify labels: rollup
This PR moves
valid_ret_activity
andvalid_input_activity
checks to the macro expansion phase in compiler/rustc_builtin_macros/src/autodiff.rs, replacing the following internal compiler error (ICE):with a more user-friendly message.
The issue specifically affected the test file
tests/ui/autodiff/autodiff_illegal.rs
, impacting the functionsf5
andf6
.The ICE can be reproduced by following Enzyme's Rustbook installation guide.
Additionally, this PR adds tests for invalid return activity in
autodiff_illegal.rs
, which previously triggered an unnoticed ICE before these fixes.r? @oli-obk