Skip to content

Trigger add_vis assist on paths/record fields as well #4273

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

Merged
merged 3 commits into from
May 14, 2020

Conversation

TimoFreiberg
Copy link
Contributor

@TimoFreiberg TimoFreiberg commented May 2, 2020

Resolves #4037.

  • Function defs
  • ADT defs
  • Enum variants
  • Consts
  • Statics
  • Traits
  • Type aliases
  • Modules
  • Record fields (using different implementation)
    • struct fields
    • enum variant fields
    • ❌ union fields (Semantics::resolve_record_field seems to not work for union fields, so I think this can be handled in a future PR)
  • More tests?
  • Improve test fixture code and documentation a bit (see Zulip)

@TimoFreiberg TimoFreiberg force-pushed the add_visibility_via_path branch 3 times, most recently from 1f9ad2d to 32dbb94 Compare May 3, 2020 20:20
@TimoFreiberg TimoFreiberg force-pushed the add_visibility_via_path branch from 32dbb94 to f35c29f Compare May 8, 2020 15:10
@TimoFreiberg TimoFreiberg changed the title WIP trigger add_vis assist on paths as well trigger add_vis assist on paths as well May 8, 2020
@TimoFreiberg TimoFreiberg changed the title trigger add_vis assist on paths as well Trigger add_vis assist on paths/record fields as well May 8, 2020
@TimoFreiberg TimoFreiberg force-pushed the add_visibility_via_path branch 2 times, most recently from 7339c2d to 91bf7f3 Compare May 8, 2020 15:30
Copy link
Member

@flodiebold flodiebold left a comment

Choose a reason for hiding this comment

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

One case where this won't work correctly is with reexports, I think -- e.g. in

mod foo {
    use bar::Baz;
    mod bar { pub(super) struct Baz; }
}
foo::Baz<|>

we'll just change the visibility on the struct declaration, which isn't enough. That's kind of hard to fix though, so I'd probably leave it for now (maybe add a test to document it).

@TimoFreiberg TimoFreiberg force-pushed the add_visibility_via_path branch from 91bf7f3 to 93038f3 Compare May 10, 2020 13:59
@TimoFreiberg TimoFreiberg force-pushed the add_visibility_via_path branch from 93038f3 to e17193d Compare May 10, 2020 14:51

#[test]
#[ignore]
// FIXME handle reexports properly
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a test for the reexport case

@matklad
Copy link
Member

matklad commented May 14, 2020

r? @flodiebold :-)

@flodiebold
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented May 14, 2020

@bors bors bot merged commit 12d8268 into rust-lang:master May 14, 2020
@TimoFreiberg TimoFreiberg deleted the add_visibility_via_path branch May 14, 2020 15:52
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request May 20, 2025
TB: add `Cell` state to support more fine-grained tracking of interior mutable data
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.

Call make public assist from invalid function call
3 participants