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.

Sign up for GitHub

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

Jump to bottom

pub(in path), pub(super), and pub(self) are broken #2398

Closed
uu1t opened this issue Jan 27, 2018 · 2 comments · Fixed by #2512
Closed

pub(in path), pub(super), and pub(self) are broken #2398

uu1t opened this issue Jan 27, 2018 · 2 comments · Fixed by #2512
Labels
bug Panic, non-idempotency, invalid code, etc.

Comments

@uu1t
Copy link

uu1t commented Jan 27, 2018

Example from Visibility and Privacy in the reference:

Before

pub mod outer_mod {
    pub mod inner_mod {
        pub(in outer_mod) fn outer_mod_visible_fn() {}
        pub(super) fn super_mod_visible_fn() {}
        pub(self) fn inner_mod_visible_fn() {}
    }
}

After

pub mod outer_mod {
    pub mod inner_mod {
        pub(in outer_mod) fn outer_mod_visible_fn() )
        {
        }
        pub(super) fn super_mod_visible_fn() )
        {
        }
        pub(self) fn inner_mod_visible_fn() )
        {
        }
    }
}

version: 0.3.6-nightly (dfc67a5d 2018-01-26)
@topecongiro topecongiro added the bug Panic, non-idempotency, invalid code, etc. label Jan 27, 2018
@topecongiro topecongiro mentioned this issue Jan 27, 2018
Merged
@ExpHP ExpHP mentioned this issue Feb 15, 2018
Open
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 16, 2018
@GuillaumeGomez
Rollup merge of rust-lang#47799 - topecongiro:fix-span-of-visibility,…
c86bc4a 
… r=petrochenkov

Fix span of visibility

This PR

1. adds a closing parenthesis to the span of `Visibility::Crate` (e.g. `pub(crate)`). The current span only covers `pub(crate`.
2. adds a `span` field to `Visibility::Restricted`. This span covers the entire visibility expression (e.g. `pub (in self)`). Currently all we can have is a span for `Path`.

This PR is motivated by the bug found in rustfmt (rust-lang/rustfmt#2398).

The first change is a strict improvement IMHO. The second change may not be desirable, as it adds a field which is currently not used by the compiler.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 17, 2018
@GuillaumeGomez
Rollup merge of rust-lang#47799 - topecongiro:fix-span-of-visibility,…
4aedf27 
… r=petrochenkov

Fix span of visibility

This PR

1. adds a closing parenthesis to the span of `Visibility::Crate` (e.g. `pub(crate)`). The current span only covers `pub(crate`.
2. adds a `span` field to `Visibility::Restricted`. This span covers the entire visibility expression (e.g. `pub (in self)`). Currently all we can have is a span for `Path`.

This PR is motivated by the bug found in rustfmt (rust-lang/rustfmt#2398).

The first change is a strict improvement IMHO. The second change may not be desirable, as it adds a field which is currently not used by the compiler.
bors added a commit to rust-lang/rust that referenced this issue Feb 19, 2018
@bors
Auto merge of #47799 - topecongiro:fix-span-of-visibility, r=petroche…
d7e45b0 
…nkov

Fix span of visibility

This PR

1. adds a closing parenthesis to the span of `Visibility::Crate` (e.g. `pub(crate)`). The current span only covers `pub(crate`.
2. adds a `span` field to `Visibility::Restricted`. This span covers the entire visibility expression (e.g. `pub (in self)`). Currently all we can have is a span for `Path`.

This PR is motivated by the bug found in rustfmt (rust-lang/rustfmt#2398).

The first change is a strict improvement IMHO. The second change may not be desirable, as it adds a field which is currently not used by the compiler.
bors added a commit to rust-lang/rust that referenced this issue Feb 23, 2018
@bors
Auto merge of #47799 - topecongiro:fix-span-of-visibility, r=petroche…
063deba 
…nkov

Fix span of visibility

This PR

1. adds a closing parenthesis to the span of `Visibility::Crate` (e.g. `pub(crate)`). The current span only covers `pub(crate`.
2. adds a `span` field to `Visibility::Restricted`. This span covers the entire visibility expression (e.g. `pub (in self)`). Currently all we can have is a span for `Path`.

This PR is motivated by the bug found in rustfmt (rust-lang/rustfmt#2398).

The first change is a strict improvement IMHO. The second change may not be desirable, as it adds a field which is currently not used by the compiler.
@nrc
Copy link
Member

nrc commented Mar 5, 2018

This should be fixed now and just needs a test

@topecongiro
Copy link
Contributor

Not quite: we need to update rustc-ap-syntax to include the fix from libsyntax.

topecongiro added a commit to topecongiro/rustfmt that referenced this issue Mar 6, 2018
@topecongiro
Add tests for rust-lang#2398
0393037
topecongiro added a commit to topecongiro/rustfmt that referenced this issue Mar 6, 2018
@topecongiro
Modify code around ast::Visibility
5416c4d 
`ast::Visibility` is changed to `codemap::Spanned` whose node is
`ast::VisibilityKind`. This commit fixes it.

Closes rust-lang#2398.
@topecongiro topecongiro mentioned this issue Mar 6, 2018
Merged
@nrc nrc closed this as completed in #2512 Mar 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update rustc-ap-syntax topecongiro/rustfmt
3 participants
@nrc @topecongiro @uu1t