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: Recover from indeterminate macro resolutions more agressively #53587

Closed
wants to merge 1 commit into from

Conversation

petrochenkov
Copy link
Contributor

If we are in "forced resolution" mode and in-module resolution is indeterminate, don't give up and continue searching in outer scopes.

Fixes #53481

If we are in "forced resolution" mode and in-module resolution is indeterminate, don't give up and continue searching in outer scopes
@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 22, 2018
@petrochenkov
Copy link
Contributor Author

r? @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 22, 2018

📌 Commit c9c79b7 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 22, 2018
@petrochenkov
Copy link
Contributor Author

Nah, this is not a good solution, easy way to get compiling code with potentially inconsistent resolutions.
Need to try something more conservative first.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 22, 2018
@petrochenkov
Copy link
Contributor Author

petrochenkov commented Aug 25, 2018

The root issue here is that derive helper attributes like #[serde] in #53481 or #[my_attr] in the test require going through forced resolution, this is something that needs to be avoided.

Ideally, forced resolution should be an error recovery measure and should never result in successful compilation, except for the feature(custom_attribute) case, but that feature is probably going to be retired eventually.

I'll return to this until beta/stable, but for now it's blocked by some other work from #50911 (comment).

@petrochenkov petrochenkov added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 25, 2018
@norru
Copy link

norru commented Aug 29, 2018

Has consensus been reached on this resolution yet? I have been unable to upgrade Nightly past 2018-08-15 (build failures on dependencies).

@petrochenkov
Copy link
Contributor Author

@norru
This PR as written will result in accepting code that needs to be rejected and thus poses a backward-compatibility risk.
The proper fix is less trivial, but I hope to implement it in the next few weeks.

In the meantime, I'd recommend to implement a workaround in log4rs (if that is the dependency you are talking about) and publish a minor version, or simply to stay on the old nightly.

@norru
Copy link

norru commented Aug 30, 2018

Hi @petrochenkov, thanks for the extensive explanation. I am not willing/keen on using a patched version of https://github.com/sfackler/log4rs so I'll stick to the old Nightly. How long do you think is going to take, for the sake of my own planning? @sfackler FYI

@nikomatsakis
Copy link
Contributor

@petrochenkov Is this PR still blocked?

@petrochenkov
Copy link
Contributor Author

This PR can be closed actually,
The only thing that can be reused from here is the test.
I hope to return to this issue this weekend or next week.
It would be nice to land refactorings from #53778 before that, but not necessary.

@Boscop
Copy link

Boscop commented Sep 13, 2018

@petrochenkov

or simply to stay on the old nightly.

I'm running into this issue with both nightly-2018-08-25 and nightly-2018-09-13.
Which old nightly would work with log4rs?
I need to use at least nightly-2018-08-25 because of rocket..

@petrochenkov
Copy link
Contributor Author

@Boscop
The issue was fixed in #54086 (merged into rust-lang:master 10 hours ago), so the fix should appear in today's or tomorrow's nightly.

@ghost
Copy link

ghost commented Sep 14, 2018

@petrochenkov thank you for being so responsive.

Everyone working with rocket and log4rs owes you one 🍺

@petrochenkov petrochenkov deleted the serdregr branch June 5, 2019 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants