Skip to content
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

Fix groups inside namespaces in build! #806

Merged
merged 3 commits into from
May 18, 2021

Conversation

fghzxm
Copy link
Contributor

@fghzxm fghzxm commented May 16, 2021

Currently BuildMacro::parse tracks the current namespace-path with a
single mutable string throughout the walking of the UseTree. When the
walking of a UseTree::Path is completed, it incorrectly forgets to pop
that sub-namespace off the namespace-path string. As a result, when a
UseTree::Group contains more than one sub-UseTrees, latter
sub-UseTrees are walked with a namespace-path that has the leaf
sub-namespaces of the previous sub-UseTrees incorrectly infixed into
it:

// builds A.B.C and A.B.D.E, should build A.B.C and A.D.E
windows::build!(
    A::{B::C, D::E}
);

This commit fixes that by cloning the namespace-path string when walking
a UseTree::Group.

It also changes the diagnostics when trying to importing a nonexistent
item in the global namespace from .Foo to (global namespace).Foo.

fghzxm and others added 2 commits May 16, 2021 14:57
Currently `BuildMacro::parse` tracks the current namespace-path with a
single mutable string throughout the walking of the `UseTree`.  When the
walking of a `UseTree::Path` is completed, it incorrectly forgets to pop
that sub-namespace off the namespace-path string.  As a result, when a
`UseTree::Group` contains more than one sub-`UseTree`s, latter
sub-`UseTree`s are walked with a namespace-path that has the leaf
sub-namespaces of the previous sub-`UseTree`s incorrectly infixed into
it:

```rust
// builds A.B.C and A.B.D.E, should build A.B.C and A.D.E
windows::build!(
    A::{B::C, D::E}
);
```

This commit fixes that by cloning the namespace-path string when walking
a `UseTree::Group`.

It also changes the diagnostics when trying to importing a nonexistent
item in the global namespace from `.Foo` to `(global namespace).Foo`.
Copy link
Collaborator

@kennykerr kennykerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! Please run cargo fmt to let the build succeed.

@fghzxm fghzxm requested a review from kennykerr May 18, 2021 02:33
@kennykerr kennykerr merged commit 53058cc into microsoft:master May 18, 2021
@fghzxm fghzxm deleted the build-groups branch May 18, 2021 04:48
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

Successfully merging this pull request may close these issues.

None yet

2 participants