Function parameters cannot shadow statics

[wouterdebie]

I'm working on a project that is based on [esp-idf-template](https://github.com/esp-rs/esp-idf-template) and when I'm trying to include esp-idf-hal in my Cargo.toml compilation fails with the following error:

error[E0530]: function parameters cannot shadow statics
  --> /Users/wouter.de.bie/.cargo/registry/src/github.com-1ecc6299db9ec823/esp-idf-hal-0.40.1/src/adc.rs:88:17
   |
58 |     use esp_idf_sys::*;
   |         -------------- the static `resolution` is imported here
...
88 |         fn from(resolution: Resolution) -> Self {
   |                 ^^^^^^^^^^ cannot be named the same as a static

error[E0530]: function parameters cannot shadow statics
   --> /Users/wouter.de.bie/.cargo/registry/src/github.com-1ecc6299db9ec823/esp-idf-hal-0.40.1/src/adc.rs:117:37
    |
58  |     use esp_idf_sys::*;
    |         -------------- the static `resolution` is imported here
...
117 |         pub fn resolution(mut self, resolution: Resolution) -> Self {
    |                                     ^^^^^^^^^^ cannot be named the same as a static

error[E0530]: function parameters cannot shadow statics
  --> /Users/wouter.de.bie/.cargo/registry/src/github.com-1ecc6299db9ec823/esp-idf-hal-0.40.1/src/can.rs:76:17
   |
52 |     use esp_idf_sys::*;
   |         -------------- the static `resolution` is imported here
...
76 |         fn from(resolution: Timing) -> Self {
   |                 ^^^^^^^^^^ cannot be named the same as a static

For more information about this error, try `rustc --explain E0530`.
error: could not compile `esp-idf-hal` due to 3 previous errors

I'm compiling on MacOS with ESP IDF v5.0.

Any idea what is wrong?

[ivmarkov]

Yeah. These are the dangers of using glob-imports (as in use esp_idf_sys::*). As the content of the module you are importing might change, you are risking name conflicts. In that particular case, it seems the esp-cam component has a static named... resolution. If that's not clear, this is not your fault, but ours, as we were lazy (me specifically) and we were glob-importing the esp-idf-sys namespace.

No easy fixes :(

• Option 1: do not glob-import esp-idf-sys anymore. Anywhere. This means, replace - in esp-idf-hal and esp-idf-sys - use esp_idf_sys::* by use esp_idf_sys as esp and then prefix all calls to functions in esp-idf-sys with esp.
• Option 2: Provide a way to put the symbols of the custom component in a separate module. As in - esp-cam should be in esp_idf_sys::esp_cam or better yet - esp_idf_sys::ext_components::esp_cam. Pros and cons whether this should be the default behavior
• Option 3: Provide a way for folks using custom components to provide a "denylist" of symbols for which bindings should not be generated

I think we need to actually do all of 1, 2 and 3, and perhaps in the following order: 2, then 3, then 1. Or 3, then 2 then 1.

Given how easy option 2 is, perhaps you can do it? The line that we fixed (removed) yesterday should be very near to where you need to do a code change.

[ivmarkov]

Oh, and you can of course always just temporarily patch esp-idf-hal's adc module and rename the resolution var in there to resolution2 or something, but I'll of course never upstream such a patch. :)

[wouterdebie]

Thanks for this! In my case option 2 is probably the best. How is this different from using bindings_module in the extra_components? (which doesn't seem to solve the problem)

Another option would be to patch the esp-camera component itself and rename resolution there. I actually do this already since esp_camera.h contains a union that leads to an anonymous struct member by bindgen. Is there an easy way of patching source code before building? Right now I'm doing this manually, but since I'd like esp-camera to be a submodule, rather than vendored code, I'd love to do this automatically.

[ivmarkov]

Thanks for this! In my case option 2 is probably the best. How is this different from using bindings_module in the extra_components? (which doesn't seem to solve the problem)

No idea, as it is not me who wrote the components support originally, so now I need to read the code of it, just like you do.

Another option would be to patch the esp-camera component itself and rename resolution there. I actually do this already since esp_camera.h contains a union that leads to an anonymous struct member by bindgen. Is there an easy way of patching source code before building? Right now I'm doing this manually, but since I'd like esp-camera to be a submodule, rather than vendored code, I'd love to do this automatically.

Just fork esp-camera then commit the fixes to your repo and then depend on your repo instead of on the original one. The cleanest approach IMO.

[wouterdebie]

Regarding bindings_module I saw your comment in #182. However, when doing this (bindings_module = "esp_cam"), I'm getting the following warning:

warning: `sigaction` redeclared with a different signature
     --> /Users/wouter.de.bie/prjs/wouter/esp32-cam-rs/target/xtensa-esp32-espidf/debug/build/esp-idf-sys-d65a20c05d8c81d5/out/bindings.rs:69163:9
      |
17728 | /     pub fn sigaction(
17729 | |         arg1: ::core::ffi::c_int,
17730 | |         arg2: *const sigaction,
17731 | |         arg3: *mut sigaction,
17732 | |     ) -> ::core::ffi::c_int;
      | |____________________________- `sigaction` previously declared here
...
69163 | /         pub fn sigaction(
69164 | |             arg1: ::core::ffi::c_int,
69165 | |             arg2: *const sigaction,
69166 | |             arg3: *mut sigaction,
69167 | |         ) -> ::core::ffi::c_int;
      | |________________________________^ this signature doesn't match the previous declaration
      |
      = note: expected `unsafe extern "C" fn(i32, *const bindings::sigaction, *mut bindings::sigaction) -> i32`
                 found `unsafe extern "C" fn(i32, *const esp_cam::sigaction, *mut esp_cam::sigaction) -> i32`
      = note: `#[warn(clashing_extern_declarations)]` on by default

[wouterdebie]

No idea, as it is not me who wrote the components support originally, so now I need to read the code of it, just like you do.

Understand :) As it seems to be narrowed down to a warning, so I'm getting somewhere :) Let me have a look to see if I can figure out what's going on here and how to fix this.

[N3xed]

• Option 2: Provide a way to put the symbols of the custom component in a separate module. As in - esp-cam should be in esp_idf_sys::esp_cam or better yet - esp_idf_sys::ext_components::esp_cam. Pros and cons whether this should be the default behavior

Maybe I can chime in for this:
When I implemented this, I also wanted to put component's bindings in different modules, but it was almost impossible (at least in a non-hacky way) to implement. And as far as I can tell this is still true today (though maybe it is possible by the new bindgen::ir module (I've only skimmed to docs a bit)).

To implement this, you would need to know which C header the bindings came from during generation and add the appropriate code to the generated bindings. There is a CargoCallbacks::include_file callback but it doesn't allow to return code to be added to the bindings.

This is the reason that separate bindgen instances are run to put extra component's bindings in a separate module (and explains the docs on the bindings_module parameter).

Hopefully, this explanation shed some light on this.

[wouterdebie]

@N3xed thanks for the context! I didn't realize that extern "C" functions can still clash, even though they are in separate modules, until I read https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/builtin/static.CLASHING_EXTERN_DECLARATIONS.html.