Skip to content

autodiff macro fails to compile when used on functions defined within other functions #184

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

Closed
sYgbpNA5UEVufjsHNwF3Xvf9ANZi5bNs opened this issue Nov 3, 2024 · 2 comments

Comments

@sYgbpNA5UEVufjsHNwF3Xvf9ANZi5bNs

output of rustc +enzyme --version --verbose:

rustc 1.84.0-nightly (1203575 2024-10-30)
binary: rustc
commit-hash: 1203575
commit-date: 2024-10-30
host: x86_64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.1

an example:

#![feature(autodiff)]
use std::autodiff::autodiff;

fn main() {
    #[autodiff(d_inner, Forward, Dual, DualOnly)]
    fn inner(x: f32) -> f32 {
        x * x
    }
}

build results in the error message (with the first line with the path manually removed)

error: autodiff must be applied to function
 --> src/main.rs:6:5
  |
6 | /     fn inner(x: f32) -> f32 {
7 | |         x * x
8 | |     }
  | |_____^

error: could not compile `enzyme-test` (bin "enzyme-test") due to 1 previous error
@ZuseZ4
Copy link
Member

ZuseZ4 commented Nov 3, 2024

Thanks for the issue report! I'm a bit short on time till Nov 15, but will look at it afterwards.
That being said it's a frontend error, and those are usually easy to fix.
Here is the PR where I merged the frontend logic: https://github.com/rust-lang/rust/pull/129458/files#diff-548c9b1b2ceb2091452377c3716d4fe1b707295b33705834a97ce4274eb82afa
The error should be in compiler/rustc_builtin_macros/src/autodiff.rs, you probably need to look at the match statements which as fallback lead to this error and add the one case that matches this inner function (and is currently not getting matched).
But otherwise I can also add handling for it after my deadlines.

Zalathar added a commit to Zalathar/rust that referenced this issue Apr 7, 2025
…ZuseZ4

fix usage of `autodiff` macro with inner functions

This PR adds additional handling into the expansion step of the `std::autodiff` macro (in `compiler/rustc_builtin_macros/src/autodiff.rs`), which allows the macro to be applied to inner functions.

```rust
#![feature(autodiff)]
use std::autodiff::autodiff;

fn main() {
    #[autodiff(d_inner, Forward, Dual, DualOnly)]
    fn inner(x: f32) -> f32 {
        x * x
    }
}
```

Previously, the compiler didn't allow this due to only handling `Annotatable::Item` and `Annotatable::AssocItem` and missing the handling of `Annotatable::Stmt`. This resulted in the rather generic error

```
error: autodiff must be applied to function
 --> src/main.rs:6:5
  |
6 | /     fn inner(x: f32) -> f32 {
7 | |         x * x
8 | |     }
  | |_____^

error: could not compile `enzyme-test` (bin "enzyme-test") due to 1 previous error
```

This issue was originally reported [here](EnzymeAD#184).

Quick question: would it make sense to add a ui test to ensure there is no regression on this?
This is my first contribution, so I'm extra grateful for any piece of feedback!! :D

r? `@oli-obk`

Tracking issue for autodiff: rust-lang#124509
Kobzol added a commit to Kobzol/rust that referenced this issue Apr 7, 2025
…ZuseZ4

fix usage of `autodiff` macro with inner functions

This PR adds additional handling into the expansion step of the `std::autodiff` macro (in `compiler/rustc_builtin_macros/src/autodiff.rs`), which allows the macro to be applied to inner functions.

```rust
#![feature(autodiff)]
use std::autodiff::autodiff;

fn main() {
    #[autodiff(d_inner, Forward, Dual, DualOnly)]
    fn inner(x: f32) -> f32 {
        x * x
    }
}
```

Previously, the compiler didn't allow this due to only handling `Annotatable::Item` and `Annotatable::AssocItem` and missing the handling of `Annotatable::Stmt`. This resulted in the rather generic error

```
error: autodiff must be applied to function
 --> src/main.rs:6:5
  |
6 | /     fn inner(x: f32) -> f32 {
7 | |         x * x
8 | |     }
  | |_____^

error: could not compile `enzyme-test` (bin "enzyme-test") due to 1 previous error
```

This issue was originally reported [here](EnzymeAD#184).

Quick question: would it make sense to add a ui test to ensure there is no regression on this?
This is my first contribution, so I'm extra grateful for any piece of feedback!! :D

r? `@oli-obk`

Tracking issue for autodiff: rust-lang#124509
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 7, 2025
Rollup merge of rust-lang#138314 - haenoe:autodiff-inner-function, r=ZuseZ4

fix usage of `autodiff` macro with inner functions

This PR adds additional handling into the expansion step of the `std::autodiff` macro (in `compiler/rustc_builtin_macros/src/autodiff.rs`), which allows the macro to be applied to inner functions.

```rust
#![feature(autodiff)]
use std::autodiff::autodiff;

fn main() {
    #[autodiff(d_inner, Forward, Dual, DualOnly)]
    fn inner(x: f32) -> f32 {
        x * x
    }
}
```

Previously, the compiler didn't allow this due to only handling `Annotatable::Item` and `Annotatable::AssocItem` and missing the handling of `Annotatable::Stmt`. This resulted in the rather generic error

```
error: autodiff must be applied to function
 --> src/main.rs:6:5
  |
6 | /     fn inner(x: f32) -> f32 {
7 | |         x * x
8 | |     }
  | |_____^

error: could not compile `enzyme-test` (bin "enzyme-test") due to 1 previous error
```

This issue was originally reported [here](EnzymeAD#184).

Quick question: would it make sense to add a ui test to ensure there is no regression on this?
This is my first contribution, so I'm extra grateful for any piece of feedback!! :D

r? `@oli-obk`

Tracking issue for autodiff: rust-lang#124509
@ZuseZ4 ZuseZ4 closed this as completed Apr 8, 2025
@ZuseZ4
Copy link
Member

ZuseZ4 commented Apr 8, 2025

closed thanks to @haenoe in rust-lang#138314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants