Stream: t-compiler/wg-rls-2.0

Topic: rust-analyzer#1243


Edwin Cheng (May 13 2019 at 13:00, on Zulip):

Just tried swaping public_macro and local_macro order did fix the issue #1243

Before:

    pub(crate) fn find_macro(&self, name: &Name) -> Option<MacroDefId> {
        self.public_macros.get(name).or(self.local_macros.get(name)).map(|it| *it)
    }

After:

    pub(crate) fn find_macro(&self, name: &Name) -> Option<MacroDefId> {
         self.local_macros.get(name).or(self.public_macros.get(name)).map(|it| *it)
    }

However, i agreed we need to be just more principled with macro resolution as @matklad said. So the question is , do i submit a PR for this swapping hotfix ? Or waiting for the better name resolution implementation ?

matklad (May 13 2019 at 13:25, on Zulip):

Could also add a test for this? I am all for PR!

Edwin Cheng (May 14 2019 at 15:41, on Zulip):

Could also add a test for this? I am all for PR!

I found that i don't actually know why that fix the issue :joy: . So i start to study how current macro name resolution works and want to try implementing instead of proper macro scopes in DefMap.

Edwin Cheng (May 14 2019 at 15:45, on Zulip):

But i have a few questions:

In collector.rs :

 fn resolve_macros(&mut self) -> ReachedFixedPoint {
       ......
        macros.retain(|(module_id, ast_id, path)| {
            .......
        });

        for (module_id, macro_call_id, macro_def_id) in resolved {
            self.collect_macro_expansion(module_id, macro_call_id, macro_def_id);
        }
        res
}

Why we do not to set back the retained macros back in self.unexpanded_macros ?

Edwin Cheng (May 14 2019 at 15:49, on Zulip):

And if i understand correctly, current implementation do not support old style external crate + #[macro_use] combo , is it correct ?

matklad (May 14 2019 at 15:49, on Zulip):

that... looks like a bug?

matklad (May 14 2019 at 15:50, on Zulip):

we should swap unexpanded macros back

Last update: Nov 19 2019 at 17:35UTC