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

Enforce default discriminant type for all representations and across editions #81

Merged
merged 3 commits into from
Sep 3, 2023

Conversation

daxpedda
Copy link
Collaborator

@daxpedda daxpedda commented Sep 3, 2023

See Zulip.

Yanking time ... cry me a river ...

@daxpedda daxpedda requested a review from ModProg September 3, 2023 07:33
@ModProg
Copy link
Owner

ModProg commented Sep 3, 2023

we could do this for all cases right? even the non C one? I feel like that would make sense, as technically also the repr(Rust) could change on an edition.

@daxpedda
Copy link
Collaborator Author

daxpedda commented Sep 3, 2023

I was having the same thought, but I would have said that would break all proc-macros out there, so we should be safe? I don't want to further ruin the code generation ...

WDYT?

@daxpedda
Copy link
Collaborator Author

daxpedda commented Sep 3, 2023

We could also ask on Zulip again?

@ModProg
Copy link
Owner

ModProg commented Sep 3, 2023

well, I'd say even the change to repr(C) would break all proc macros.

@daxpedda daxpedda changed the title Enforce C representation discriminant type across editions Enforce default discriminant type for all representations and across editions Sep 3, 2023
@daxpedda
Copy link
Collaborator Author

daxpedda commented Sep 3, 2023

It's done, checking both now.

@daxpedda
Copy link
Collaborator Author

daxpedda commented Sep 3, 2023

I think we can get this out first and figure out #83 later when we get an answer.

@daxpedda daxpedda merged commit be7a2e3 into ModProg:main Sep 3, 2023
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.

2 participants