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

Implement initial version of cfg(accessible(..)) #137113

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

crlf0710
Copy link
Member

Implements an initial version of cfg(accessible(..)). The code includes a few FIXMEs, and i think i need some help with understanding how things suppose to work before i can solve them. Also, maybe i need some advice on organizing these code.

I'll assign to Vadim but feel free to reassign.

cc #64797

r? @petrochenkov

@crlf0710 crlf0710 added the F-cfg_accessible `#![feature(cfg_accessible)]` label Feb 16, 2025
@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 16, 2025

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@Urgau
Copy link
Member

Urgau commented Feb 16, 2025

This doesn't seems to share any code with the already implemented cfg_accessible macro.

What's the plan with that earlier attempt? Replace it with this one?

@crlf0710
Copy link
Member Author

What's the plan with that earlier attempt?

I guess we can decide that after the we work out the "advice on organizing these code" part :)

Copy link
Contributor

Choose a reason for hiding this comment

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

There seem to be existing tests at tests/ui/conditional-compilation/cfg_accessible-*, maybe these should stay in the same place? Or just make a new cfg-accessible directory and move everything there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack, will move those files in the next push. Any advice on the implementation?

Copy link
Contributor

@tgross35 tgross35 Feb 18, 2025

Choose a reason for hiding this comment

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

I don't know too much about this area of the compiler unfortunately, petrochenkov is probably the expert here. Just happened to notice the existing tests while doing a driveby of this feature's history.

@bors
Copy link
Contributor

bors commented Feb 25, 2025

☔ The latest upstream changes (presumably #135726) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) F-cfg_accessible `#![feature(cfg_accessible)]` S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants