Skip to content

Yet another glob import issue #19224

Open
@RivenSkaye

Description

@RivenSkaye

Previously opened as rust-lang/rust#122682 but moved here as requested. Some things have changed since, but they're not better. Old behavior in strikethrough because it might provide some pointers on what changed behavior is in play here?
Seems to only be a problem with the VSCode extension now at least. I'm not getting this error reproduced in helix.

RA breaks on certain struct definitions from external crates, due to #[repr(transparent)] across the crate boundary. As a "fix" it makes you go back and forth between the outer type and the transparently wrapped one wrap it in the transparent type, but specifically in VSCode that errors claiming to expect the inner type. In the example provided near the bottom, these are HRESULT and i32.

This doesn't feel ergonomic, and more importantly the type resolution provided is broken to the point it's actively confusing. Where I'd expect a clearer resolution or no actions available, the quick fix advises users to change wrapper -> underlying type and vice versa use the transparent wrapper, and then it errors with no fix available. This creates a very confusing situation. Subsequently verifying the type signature through the VSCode plugin doesn't help, as it lists the underlying type rather than the transparent wrapper.

Image
Image
For this one, it does suggest making it a HRESULT, but then we go back to the previous screenshot.
Image

The only solution that appeases the RA extension without compilation issues is HRESULT::Into<i32>.

rust-analyzer version: rust-analyzer 1.87.0-nightly (5bc62314 2025-02-16) with VSCode extension. Both pre-release (currently version 0.4.2320), and release (currently version 0.3.2319) channels.

rustc version: rustc 1.87.0-nightly (5bc623145 2025-02-16)

editor or extension: VSCode, both bundled RA and the one gotten through rustup

relevant settings: Tried with both the RA extension's bundled RA as well as the toolchain's.

  • CARGO_HOME=/home/martin/.rust/.cargo
  • RUSTUP_HOME=/home/martin/.rust

For helix the config is untouched
Relevant VSCode settings are added to the bottom because code blocks in spoilers break.
There is no global RA config. Only some personal rustmft for line lengths and other hills not to die on.

repository link (if public, optional): -

code snippet to reproduce:

Only one dependency needed. Don't know of any other crates with similar transparent structs by heart, and the bug doesn't reproduce within a crate or workspace.

[package]
    edition = "2024"
    name    = "ra-repr-mre"
    version = "0.1.0"

[dependencies]
    windows = {version = "0.60", features = ["Win32_Foundation"]}
use windows::core::{Error as WErr, HRESULT};

fn some_res(succeed: i32) -> Result<(), WErr> {
    match succeed {
        1 => Err(WErr::new(HRESULT(succeed), "This doesn't work")), // Error: expected an i32
        2 => Err(WErr::new(succeed, "Neither does this")),          // Error: expected a HRESULT. This also blocks compilation, and that seems like correct behavior to me
        // 3 and 255 removed, they added nothing of value.
        0 => Ok(()),
        _ => Err(WErr::new(HRESULT(succeed).into(), "This is what you need for it to work")),
    }
}

fn main() {
    some_res(1).expect("If you get here, compilation succeeded")
}

VSC settings, I've tweaked several of these to no avail.

"[rust]": {
        "editor.defaultFormatter": "rust-lang.rust-analyzer",
        "editor.formatOnSave": true,
        "editor.formatOnSaveMode": "file",
        "editor.inlayHints.enabled": "on"
    },
    "rust-analyzer.cargo.autoreload": true,
    "rust-analyzer.checkOnSave": true,
    "rust-analyzer.diagnostics.enable": true,
    "rust-analyzer.highlightRelated.breakPoints.enable": true,
    "rust-analyzer.highlightRelated.exitPoints.enable": true,
    "rust-analyzer.highlightRelated.references.enable": true,
    "rust-analyzer.highlightRelated.yieldPoints.enable": true,
    "rust-analyzer.hover.actions.enable": true,
    "rust-analyzer.hover.documentation.enable": true,
    "rust-analyzer.hover.links.enable": true,
    "rust-analyzer.imports.granularity.enforce": true,
    "rust-analyzer.imports.granularity.group": "crate",
    "rust-analyzer.imports.group.enable": true,
    "rust-analyzer.imports.merge.glob": false,
    "rust-analyzer.joinLines.joinAssignments": true,
    "rust-analyzer.restartServerOnConfigChange": true,
    "rust-analyzer.server.path": "rust-analyzer.exe",
    "rust-analyzer.signatureInfo.detail": "full",
    "rust-analyzer.signatureInfo.documentation.enable": true,
    "rust-analyzer.typing.autoClosingAngleBrackets.enable": true,
    "rust-analyzer.workspace.symbol.search.kind": "all_symbols",
    "rust-analyzer.workspace.symbol.search.scope": "workspace_and_dependencies",
    "rust-analyzer.semanticHighlighting.punctuation.specialization.enable": true,

Activity

flodiebold

flodiebold commented on Feb 25, 2025

@flodiebold
Member

If HRESULT is defined as shown in the screenshot, i.e. struct HRESULT(i32), then RA's type error is correct. #[repr(transparent)] doesn't mean that an i32 would be implicitly converted to it. So apparently rust-analyzer and rustc disagree about the definition of HRESULT there; maybe the windows crate does some weird conditional thing there? I haven't looked yet.

flodiebold

flodiebold commented on Feb 25, 2025

@flodiebold
Member

The windows crate explicitly exports the newtype version:
https://github.com/microsoft/windows-rs/blob/c9ae41f507f41bf45d922b11b036f17435c419cc/crates/libs/result/src/lib.rs#L32
and the Error::new definition uses HRESULT via use super::*:
https://github.com/microsoft/windows-rs/blob/c9ae41f507f41bf45d922b11b036f17435c419cc/crates/libs/result/src/error.rs#L91
So I'm still confused why it would come up with the i32 type alias instead of the newtype.

It even has an impl for HRESULT, which would not be allowed for the type alias:
https://github.com/microsoft/windows-rs/blob/master/crates/libs/result/src/error.rs#L159

flodiebold

flodiebold commented on Feb 25, 2025

@flodiebold
Member

Ah wait, I was reading the error messages wrong and RA is the one defining HRESULT as the type alias, not the other way around. Yeah then this seems to be a problem with our handling of glob imports.

RivenSkaye

RivenSkaye commented on Feb 25, 2025

@RivenSkaye
Author

Oh, my bad on not including the info. The relevant HRESULT is defined as:

/// An error code value returned by most COM functions.
#[repr(transparent)]
#[derive(Copy, Clone, Default, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[must_use]
#[allow(non_camel_case_types)]
pub struct HRESULT(pub i32);

impl<T> From<Result<T>> for HRESULT {
    fn from(result: Result<T>) -> Self {
        if let Err(error) = result {
            return error.into();
        }
        HRESULT(0)
    }
}

// skipped impl HRESULT for brevity, see https://github.com/microsoft/windows-rs/blob/master/crates/libs/result/src/hresult.rs for impl

Where the From<Result<_,_>> implementations seem to only be available for the nearly identical (and non-public) structs NTSTATUS and WIN32_ERROR, as well as From<HRESULT> being there. I'll try and see if I can set up a more minimal MRE, possibly something that leverages local paths (but not workspaces) for deps

flodiebold

flodiebold commented on Feb 25, 2025

@flodiebold
Member

well the relevant code seems to be basically this:

mod bindings {
    pub type HRESULT = i32;
}
use bindings::*;
mod hresult {
    pub struct HRESULT(i32);
}
pub use hresult::*;
pub use hresult::HRESULT;
mod error {
    use super::*;
    pub fn new_error(result: HRESULT) {}
}
fn test() {
    error::new_error(HRESULT(0));
}

but this doesn't seem to be enough to reproduce the problem in my RA, so maybe I'm missing something else.
(I can't try to reproduce the original problem since I'm not on Windows.)

ChayimFriedman2

ChayimFriedman2 commented on Feb 26, 2025

@ChayimFriedman2
Contributor

Minimal reproduction:

mod bindings {
    pub type HRESULT = i32;
}
use bindings::*;
mod error {
    use super::*;
    pub fn nonzero_hresult(hr: HRESULT) {
        _ = hr.0;
    }
}
mod hresult {
    pub struct HRESULT(pub i32);
}
pub use hresult::HRESULT;

This is because the pub use hresult::HRESULT comes too late. If we move it above mod error, it works.

Veykril

Veykril commented on Feb 26, 2025

@Veykril
Member

One day we will handle glob imports completely correct

ChayimFriedman2

ChayimFriedman2 commented on Feb 26, 2025

@ChayimFriedman2
Contributor

That day is far in the future, I fear 😂

If you are making a new language, choose between glob imports and cyclic modules. Having both leads to so many bugs.

RivenSkaye

RivenSkaye commented on Feb 26, 2025

@RivenSkaye
Author

rustc and cargo check don't have problems with this. Is there any specific reason that their approach can't be copied here? And is there any reason the exact same RA binary isn't producing this warning on the CLI or through helix?

ChayimFriedman2

ChayimFriedman2 commented on Feb 26, 2025

@ChayimFriedman2
Contributor

rustc had a lot more time and effort spent fixing its name resolution problems (and copying what it does is indeed what I intend to do to fix this issue).

I don't know about Helix, but how are you running rust-analyzer CLI? (Edit: Maybe because the mismatched types error is experimental?)

RivenSkaye

RivenSkaye commented on Feb 26, 2025

@RivenSkaye
Author

how are you running rust-analyzer CLI?

The troubleshooting guide's recommended rust-analyzer analysis-stats .

Edit: Maybe because ...

Could be, but I deleted the MRE code (and have long since changed the code that even led me to this issue) so I'm not in any position to check right now

5 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

A-nameresname, path and module resolutionC-bugCategory: bug

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @flodiebold@Veykril@RivenSkaye@AndreySmirnov81@ChayimFriedman2

      Issue actions

        Yet another glob import issue · Issue #19224 · rust-lang/rust-analyzer