-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add more rust_private attributes #56559
Conversation
253e73f
to
00367d5
Compare
I think this is probably indicative of other problems perhaps? These attributes shouldn't be necessary due to how rustbuild builds the compiler crates, and this attribute would otherwise be needed by a much larger number of crates. I think that clippy integration into rustc will be somewhat difficult, but it'll likely for sure need some tight integration with rustbuild itself! |
I am trying an approach similar to |
@alexcrichton An additional effect of this change is the improved ability to execute |
Is this still necessary or has it largely moved over to #56595? |
It is still needed in order for all components to work with |
Hm well as I mentioned before these feature gates should not be necessary, and indicates a bug in the build setup if they're otherwise required. I haven't had a chance to dig into the other PR yet, but if it requires this one then I think that the other PR needs some more work |
☔ The latest upstream changes (presumably #56614) made this pull request unmergeable. Please resolve the merge conflicts. |
ping from triage @alexcrichton: Could you clarify you previous review? Is this PR entirely infeasible or just needs some additional work? |
Oh sorry sure, this PR shouldn't be necessary (and we're building just fine AFAIK today), so I think there may have been something local giong wrong? In any case I'll go ahead and close. |
I'm working on enabling clippy in
x.py
and the usage ofis_xid_start
andis_xid_continue
in the components is one of the obstacles. If we are not planning to remove this attribute then it makes sense to use it for these compiler internals, as it is already done with some of the others.If for any reason this is undesirable, there is probably some way of working around it; otherwise it seems like the most straightforward option.
r? @alexcrichton