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

#[!resolve_unexported] => #[resolve_unexported] to fix pretty printing #15115

Closed
wants to merge 1 commit into from

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Jun 23, 2014

Without this change, we were unable to do:

% rustc --test --pretty expanded foo.rs > bar.rs
% rustc bar.s

Without this change, we were unable to do:

% rustc --test --pretty expanded foo.rs > bar.rs
% rustc bar.s
@alexcrichton
Copy link
Member

I believe that the name of this attribute was intentionally chosen so it could never be written down. This attribute allows a module to entirely bypass all privacy restrictions which is a little worrisome to allow on a general basis.

@alexcrichton
Copy link
Member

That being said, I would very much like to say --test --pretty expanded and have it "just work". I'm not entirely sure how to do it without breaking privacy, though...

@erickt
Copy link
Contributor Author

erickt commented Jun 27, 2014

Hm, I didn't realize that's what it was doing. Having an attribute that does that feels a little icky. Is that mainly so we can write:

#[test]
fn test_foo() { ... }

Instead of:

#[test]
pub fn test_foo() { ... }

? If so, i don't think adding pub is that painful, so I question the worth of #[!resolve_unexported]. However, if there is a case I'm not thinking of, I suppose we could put some warning flags around it, with something like #[__unsafe_unstable_resolve_unexported] or something.

@huonw
Copy link
Member

huonw commented Jun 27, 2014

@erickt the whole module path has to be pub, not just the #[test] fn itself.

@alexcrichton
Copy link
Member

Another possible solution to this problem would be to just not print the module with --pretty expanded --test. That would require passing --test both times, but it would make the expanded source equivalent?

@klutzy
Copy link
Contributor

klutzy commented Jun 28, 2014

Yet another possible approach: rename !resolve_unexported into test.
rustc strips #[test]/#[bench] codes if --test is not given, so the expanded code will be valid.
(Or conversely, we may add resolve_unexported to rustc::front::strip_test_functions.)

@erickt
Copy link
Contributor Author

erickt commented Jul 10, 2014

@klutzy's idea of adding resolve_unexported to rustc::front::strip_test_functions seems pretty reasonable to me. How do the rest of you feel about this solution?

@alexcrichton
Copy link
Member

This seems isolated to pretty printing, and I would rather just not print the #![resolve_unexported] module during pretty printing.

If pretty printing succeeds, then you'll have two top-level modules for running the test harness, and likely two #[main] functions, so you may want to verify that #[resolve_unexported] works before printing it?

@lilyball
Copy link
Contributor

This PR is obsoleted by #15847, which removes the need for #[!resolve_unexported].

@alexcrichton
Copy link
Member

Closing in favor of #15847.

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.

5 participants