Skip to content

Analysis confused by proc-macro expansion with input very similar to syntactically valid enum? #10463

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
EndilWayfare opened this issue Oct 5, 2021 · 12 comments

Comments

@EndilWayfare
Copy link

EndilWayfare commented Oct 5, 2021

Apologies if this falls under purview of known proc-macro limitations. From my limited perspective, it doesn't seem to.

A proc macro that generates valid enums from "enum-like" syntax seems to trip up rust-analyzer

#[test]
fn qualified() {
    oxidize_enum! {
        enum DocumentKind {
            // macro resolves item paths for sweet `Into`/`From` shenanigans.
            // "Qualified path" supported in addition to "imported identifier only" for convenience, flexibility, and hygiene
            // Multi-segment paths are not valid Variant names, though. Pre-expansion, syntax is invalid.
            swconst::swDocNONE,
            swconst::swDocPART,
            swconst::swDocASSEMBLY,
            swconst::swDocDRAWING,
            swconst::swDocSDM,
            swconst::swDocLAYOUT,
            swconst::swDocIMPORTED_PART,
            swconst::swDocIMPORTED_ASSEMBLY,
        }
    }

    // `rust-analyzer` gets hung up on the enum "defined" literally in the source code,
    // doesn't operate on the expanded version.
    //
    // It thinks this `match` is not exhaustive,
    // additionally expecting as many non-existent `swconst` arms as actual variants.
    // `rustc` compiles it without issue, as I would expect.
    match DocumentKind::swDocNONE {
        DocumentKind::swDocNONE
        | DocumentKind::swDocPART
        | DocumentKind::swDocASSEMBLY
        | DocumentKind::swDocDRAWING
        | DocumentKind::swDocSDM
        | DocumentKind::swDocLAYOUT
        | DocumentKind::swDocIMPORTED_PART
        | DocumentKind::swDocIMPORTED_ASSEMBLY => true,
    };
}

// First test broadly equivalent to the following:
#[test]
fn local() {
    // Liberal macro syntax intended to avoid this kind of namespace pollution:
    use swconst::*;

    oxidize_enum! {
        enum DocumentKind {
            swDocNONE,
            swDocPART,
            swDocASSEMBLY,
            swDocDRAWING,
            swDocSDM,
            swDocLAYOUT,
            swDocIMPORTED_PART,
            swDocIMPORTED_ASSEMBLY,
        }
    }

    match DocumentKind::swDocNONE {
        DocumentKind::swDocNONE
        | DocumentKind::swDocPART
        | DocumentKind::swDocASSEMBLY
        | DocumentKind::swDocDRAWING
        | DocumentKind::swDocSDM
        | DocumentKind::swDocLAYOUT
        | DocumentKind::swDocIMPORTED_PART
        | DocumentKind::swDocIMPORTED_ASSEMBLY => true,
    };
}

Simply removing the "enum" token from both variations (and removing the associated proc macro parsing) works just fine. This seems to indicate that rust-analyzer is capable of expanding and analyzing an proc macro that generates an enum, just that -- for some reason -- it gets hung up if the macro input looks sufficiently like an enum before expansion.

I initially included the "enum" token as part of the macro input following the pattern established by winapi::ENUM macro. It doesn't generate an enum (but only a type alias from what would be the enum's identifier and constants (not syntactically related, and thus not compile-time checkable) for each variant), so it doesn't confuse rust-analyzer. The whole point of my macro, though, is to generate a Rust enum for better ergonomics in higher-level bindings to Microsoft COM components. It uses the outputs of the winapi::ENUM macro (manually collected), because bindgen'd enum values.

It's a simple enough fix for my purposes simply to continue eliding the "enum" token from my macro input.

The behavior here, though, seems like it might mess other people's macros up, though. What if a macro is written that takes a valid enum as input but modifies it into another valid enum? It seems that rust-analyzer might also incorrectly base its analysis on the pre-expansion valid macro.

Also, it seems to indicate a deficiency in rust-analyzer's syntax parsing: rustc outright rejects enums with invalid variant names while rust-analyzer seems to accept the deviant syntax just enough to confusedly generate phantom variants. Presumably, rustc's compile error usually steps in first, so rust-analyzer doesn't normally see the degenerate enum. But here, where rustc just passes it along to the proc macro expander, that sieve doesn't apply.

@bjorn3
Copy link
Member

bjorn3 commented Oct 5, 2021

Can you post the exact expansion of the macro invocation in your example? You can use cargo expand if you first did cargo install cargo-expand and switch to nightly.

@EndilWayfare
Copy link
Author

Expansion
#![feature(prelude_import)]
#![allow(non_camel_case_types, non_upper_case_globals)]
#[prelude_import]
use std::prelude::rust_2018::*;
#[macro_use]
extern crate std;
use glazing_macros::oxidize_enum;
mod swconst {
    use winapi::ENUM;
    pub type swDocumentTypes_e = u32;
    pub const swDocNONE: swDocumentTypes_e = 0;
    pub const swDocPART: swDocumentTypes_e = 1;
    pub const swDocASSEMBLY: swDocumentTypes_e = 2;
    pub const swDocDRAWING: swDocumentTypes_e = 3;
    pub const swDocSDM: swDocumentTypes_e = 4;
    pub const swDocLAYOUT: swDocumentTypes_e = 5;
    pub const swDocIMPORTED_PART: swDocumentTypes_e = 6;
    pub const swDocIMPORTED_ASSEMBLY: swDocumentTypes_e = 7;
}
extern crate test;
#[cfg(test)]
#[rustc_test_marker]
pub const qualified: test::TestDescAndFn = test::TestDescAndFn {
    desc: test::TestDesc {
        name: test::StaticTestName("qualified"),
        ignore: false,
        allow_fail: false,
        compile_fail: false,
        no_run: false,
        should_panic: test::ShouldPanic::No,
        test_type: test::TestType::IntegrationTest,
    },
    testfn: test::StaticTestFn(|| test::assert_test_result(qualified())),
};
fn qualified() {
    enum DocumentKind {
        swDocNONE,
        swDocPART,
        swDocASSEMBLY,
        swDocDRAWING,
        swDocSDM,
        swDocLAYOUT,
        swDocIMPORTED_PART,
        swDocIMPORTED_ASSEMBLY,
    }
    match DocumentKind::swDocNONE {
        DocumentKind::swDocNONE
        | DocumentKind::swDocPART
        | DocumentKind::swDocASSEMBLY
        | DocumentKind::swDocDRAWING
        | DocumentKind::swDocSDM
        | DocumentKind::swDocLAYOUT
        | DocumentKind::swDocIMPORTED_PART
        | DocumentKind::swDocIMPORTED_ASSEMBLY => true,
    };
}
extern crate test;
#[cfg(test)]
#[rustc_test_marker]
pub const local: test::TestDescAndFn = test::TestDescAndFn {
    desc: test::TestDesc {
        name: test::StaticTestName("local"),
        ignore: false,
        allow_fail: false,
        compile_fail: false,
        no_run: false,
        should_panic: test::ShouldPanic::No,
        test_type: test::TestType::IntegrationTest,
    },
    testfn: test::StaticTestFn(|| test::assert_test_result(local())),
};
fn local() {
    use swconst::*;
    enum DocumentKind {
        swDocNONE,
        swDocPART,
        swDocASSEMBLY,
        swDocDRAWING,
        swDocSDM,
        swDocLAYOUT,
        swDocIMPORTED_PART,
        swDocIMPORTED_ASSEMBLY,
    }
    match DocumentKind::swDocNONE {
        DocumentKind::swDocNONE
        | DocumentKind::swDocPART
        | DocumentKind::swDocASSEMBLY
        | DocumentKind::swDocDRAWING
        | DocumentKind::swDocSDM
        | DocumentKind::swDocLAYOUT
        | DocumentKind::swDocIMPORTED_PART
        | DocumentKind::swDocIMPORTED_ASSEMBLY => true,
    };
}
#[rustc_main]
pub fn main() -> () {
    extern crate test;
    test::test_main_static(&[&qualified, &local])
}

@EndilWayfare
Copy link
Author

Yeah, good point. I have been using cargo expand for debugging, and that's partially how I knew the macro "actually" expanded the way I expected (beyond that rustc also compiles it with resulting behavior I would expect). Wasn't sure how useful the full expansion would be in the thread, though.

@flodiebold
Copy link
Member

flodiebold commented Oct 5, 2021

What does rust-analyzer's "Expand macro recursively" command show? What's the actual error message you're getting?

(It's rather unlikely that the problem is actually the macro input looking like an enum. RA doesn't parse macro inputs like that. So it's probably more likely that there's some problem with how RA is expanding the macro.)

Edit: Also, where does "Go to definition" on the DocumentKind and/or on swDocNONE in the match take you?

@EndilWayfare
Copy link
Author

It's rather unlikely that the problem is actually the macro input looking like an enum. RA doesn't parse macro inputs like that.

That's what I thought initially. It seemed very unlikely that it would be set up that way. But, I couldn't think of any other way it could possibly be picking up swconst as (multiple?) match arms, since that token appears nowhere in the expansion.

@EndilWayfare
Copy link
Author

Hmm...

// Recursive expansion of oxidize_enum! macro
// ===========================================

enum DocumentKind {
  swDocNONE,swDocPART,swDocASSEMBLY,swDocDRAWING,swDocSDM,swDocLAYOUT,swDocIMPORTED_PART,swDocIMPORTED_ASSEMBLY
}

Odd, that looks correct...

@EndilWayfare
Copy link
Author

The initial "Fill match arms" did this:

match DocumentKind::swDocNONE {
    DocumentKind::swconst => todo!(),
    DocumentKind::swDocNONE => todo!(),
    DocumentKind::swconst => todo!(),
    DocumentKind::swDocPART => todo!(),
    DocumentKind::swconst => todo!(),
    DocumentKind::swDocASSEMBLY => todo!(),
    DocumentKind::swconst => todo!(),
    DocumentKind::swDocDRAWING => todo!(),
    DocumentKind::swconst => todo!(),
    DocumentKind::swDocSDM => todo!(),
    DocumentKind::swconst => todo!(),
    DocumentKind::swDocLAYOUT => todo!(),
    DocumentKind::swconst => todo!(),
    DocumentKind::swDocIMPORTED_PART => todo!(),
    DocumentKind::swconst => todo!(),
    DocumentKind::swDocIMPORTED_ASSEMBLY => todo!(),
};

@EndilWayfare
Copy link
Author

Umm, no idea why... but it's working now?

I put the "enum" token-supporting bit back in to get the recursive expansion... and I just noticed that rust-analyzer no longer complains?

Aw, dang it, maybe it updated when I started VS Code back up this morning? I probably should have checked for that yesterday...

@EndilWayfare
Copy link
Author

I think it might not be detecting changes in proc macro source code properly, so it's not refreshing them?

I'm having persistent issues where changing the syn::Parse implementation of my input struct isn't propagating to my tests (integration tests in "tests" folder, not in-line mod tests unit tests, if it's relevant). It spits out macro-errors that would make sense under the old macro version, but not with the most current changes. cargo test compiles successfully and says everything's fine.

Rust Analyzer: Reload workspace isn't enough. I have to Rust Analyzer: Restart server to make the errors go away. So, a similar effect to closing and relaunching VS Code. Maybe that's why it "magically" fixed itself the next day? I was pretty sure VS Code and rust-analyzer were already updated to the most current version.

@EndilWayfare
Copy link
Author

It seems possible that this problem might be related to my environment. Didn't seem relevant before, as I assumed a logic error, so here goes:

Windows 10
VS Code 1.61.0
rust-analyzer 0.2.776
rustc 1.53.0

@lnicola
Copy link
Member

lnicola commented Oct 14, 2021

I think it might not be detecting changes in proc macro source code properly, so it's not refreshing them?

That's quite likely, see #10027 and #9932.

@EndilWayfare
Copy link
Author

Ah, seems likely. Ok, so this is already a known issue. Thanks!

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

4 participants