Skip to content

[1.30 beta] Ambiguous macro names #54472

Closed

Description

@emilyalbini

Some crates in 1.30 beta are failing with ambiguous macro names errors:

 error[E0659]: `run_cancel` is ambiguous
    --> tests/pipeable.rs:156:5
     |
 156 |     run_cancel!(Simple(mock_must_cancel()));
     |     ^^^^^^^^^^ ambiguous name
     |
 error[E0659]: `try` is ambiguous
   --> src/lib.rs:52:10
    |
 52 | #[derive(Serialize, Deserialize, Debug)]
    |          ^^^^^^^^^ ambiguous name
    |

cc @petrochenkov

Activity

  1. added
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    C-bugCategory: This is a bug.
    on Sep 22, 2018
  2. added this to the 1.30 milestone on Sep 22, 2018
  3. ipetkov commented on Sep 22, 2018

    @ipetkov
    Contributor

    Only the conch-runtime tests were affected by this, which were fixed a while back but I didn't end up releasing a new version (an oversight which has now been corrected)

  4. self-assigned this
    on Sep 22, 2018
  5. petrochenkov commented on Sep 26, 2018

    @petrochenkov
    Contributor

    All the ambiguities are legitimate, sorry :(
    This is all fallout from enabling macro modularization in general.

    I think we may be able to reliably disambiguate some of these in the future, but that's not something that can be backported to beta.

    One low-hanging fruit is treating glob imports from other crates as single imports for the purposes of restricted shadowing.
    Cross-crate globs are available immediately and don't require going through fix-point, so it's not necessary to apply restricted shadowing to them.
    I'll try to implement it and then we can think about backporting.

  6. petrochenkov commented on Sep 26, 2018

    @petrochenkov
    Contributor

    One low-hanging fruit is treating glob imports from other crates as single imports for the purposes of restricted shadowing.

    It actually won't help with regressions here, I've misinterpreted the error messages.

    The ambiguities in listed crates are "macro_rules definition" vs "macro in module", so they are not caused by restricted glob shadowing.
    They need some disambiguation scheme similar to "let variable" vs "item in block", but at module level.

    fn main() {
        let x = || 0;
        println!("{:?}", x());
        {
            println!("{:?}", x());
            fn x() -> i32 { 1 }
            println!("{:?}", x());
            let x = || 2;
            println!("{:?}", x());
        }
        println!("{:?}", x());
    }
    ---
    0
    1
    1
    2
    0
  7. petrochenkov commented on Sep 27, 2018

    @petrochenkov
    Contributor

    Partially fixed in #54605 using subset of the let rules described in the previous comment.

    EDIT: I'm planning to extend #54605 to the full fix so far, but haven't yet looked what exactly causes smpl_jwt and off_blockway to fail.

  8. nagisa commented on Sep 27, 2018

    @nagisa
    Member

    @petrochenkov should we also warn when resolving in favour of macro_rules!?

  9. petrochenkov commented on Sep 27, 2018

    @petrochenkov
    Contributor

    I don't think so? In all the regressed cases the macro_rules resolution is expected, and modularized resolutions are accidental - either from globs, or from #[macro_export] planted into the root module.

    It's not something to-be-deprecated/hacky either - I planned to implement the scoping rules from #54472 (comment) eventually anyway since it's kinda "natural" expected behavior for let-like things, the regressions just made the matter more urgent.

  10. petrochenkov commented on Sep 27, 2018

    @petrochenkov
    Contributor

    Minimized reproduction for smpl_jwt and off_blockway:

    use std::*;
    
    fn main() {
        macro_rules! try { ($expr:expr) => () }
        
        try!(0);
    }
  11. petrochenkov commented on Oct 2, 2018

    @petrochenkov
    Contributor

    #54605 is updated with a fix for the remaining regressions.

  12. added 2 commits that reference this issue on Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P-highHigh priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-betaPerformance or correctness regression from stable to beta.

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions