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

Add binary_format to rustc target specs #135724

Open
Noratrieb opened this issue Jan 19, 2025 · 8 comments · May be fixed by #136637
Open

Add binary_format to rustc target specs #135724

Noratrieb opened this issue Jan 19, 2025 · 8 comments · May be fixed by #136637
Assignees
Labels
A-target-specs Area: Compile-target specifications A-targets Area: Concerning the implications of different compiler targets C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Noratrieb
Copy link
Member

Noratrieb commented Jan 19, 2025

There are some places in rustc that need to know the binary format of the target (to generate object files). It would be good to have this as a property on the target spec to avoid needing to have ad-hoc logic for it.

See #135695 (comment)

@Noratrieb Noratrieb added A-target-specs Area: Compile-target specifications A-targets Area: Concerning the implications of different compiler targets C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 19, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 19, 2025
@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 19, 2025
@yegeunyang

This comment has been minimized.

@yegeunyang

This comment has been minimized.

@Pyr0de
Copy link
Contributor

Pyr0de commented Feb 4, 2025

Hey,
I would like to give this issue a go. Could you please provide me with some more info about it?

Do I need to add a new field in Target that has the BinaryFormat?

@Noratrieb
Copy link
Member Author

Yes. You need to create a new field in the target with the binary format (create a new enum for this, don't use the one from the object crate). You will need to do a few other things as well but you may be able to figure that out from test failures and other code in the area.

To populate them for existing targets, you can find the existing logic where I linked it.

@Pyr0de
Copy link
Contributor

Pyr0de commented Feb 6, 2025

For the new enum, should it contain all the enum variants from BinaryFormat(object crate) or just the ones given in the logic mentioned in the OP?

To populate the existing targets, do I manually set the binary_format for each target in compiler/rustc_target/src/spec/targets or can I just use a function and call it in each file?

@rustbot claim

@Noratrieb
Copy link
Member Author

You should manually populate it, since the target is exactly to stop having this function. And only add the variants that we actually need.

@Pyr0de Pyr0de linked a pull request Feb 6, 2025 that will close this issue
@Pyr0de
Copy link
Contributor

Pyr0de commented Feb 6, 2025

Can the binary format be added to TargetOptions instead of Target since the logic is also based on TargetOptions?

@Noratrieb
Copy link
Member Author

Yes, I'd say put it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-target-specs Area: Compile-target specifications A-targets Area: Concerning the implications of different compiler targets C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants