Skip to content

Fix ICE when trying to compile enums with non-const discriminants. #3681

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.

Sign up for GitHub

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

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

Fix ICE when trying to compile enums with non-const discriminants. #3681

wants to merge 1 commit into from

Conversation

sakupan102
Copy link
Contributor

@sakupan102 sakupan102 commented Apr 2, 2025 •
edited
Loading

addresses #3635

gcc/rust/ChangeLog:

	* resolve/rust-ast-resolve-item.cc (ResolveItem::visit):
		add name resolution processing for discriminant values
	* resolve/rust-ast-resolve-stmt.h:
		add name resolution processing for discriminant values

gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: Exclude enum_discriminant2.rs due to ICE
	* rust/compile/enum_discriminant2.rs: New test.
	* rust/compile/enum_discriminant3.rs: New test.

@sakupan102 sakupan102 marked this pull request as draft April 2, 2025 09:08
philberty
philberty approved these changes Apr 2, 2025
View reviewed changes
Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

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

Good find LGTM

@sakupan102 sakupan102 marked this pull request as ready for review April 3, 2025 15:25
philberty
philberty approved these changes Apr 4, 2025
View reviewed changes
Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

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

LGTM

@philberty philberty added this pull request to the merge queue Apr 4, 2025
@philberty philberty removed this pull request from the merge queue due to a manual request Apr 4, 2025
philberty
philberty requested changes Apr 4, 2025
View reviewed changes
Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

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

Actually can you squash these two commits they should just be a single patch for this.

@sakupan102
add name resolution processing for discriminant values
6456726 
gcc/rust/ChangeLog:

	* resolve/rust-ast-resolve-item.cc (ResolveItem::visit):
		add name resolution processing for discriminant values
	* resolve/rust-ast-resolve-stmt.h:
		add name resolution processing for discriminant values

gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: Exclude enum_discriminant2.rs due to ICE
	* rust/compile/enum_discriminant2.rs: New test.
	* rust/compile/enum_discriminant3.rs: New test.

Signed-off-by: Ryutaro Okada <[email protected]>
@sakupan102 sakupan102 requested a review from philberty April 5, 2025 09:58
philberty
philberty approved these changes Apr 16, 2025
View reviewed changes
Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

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

LGTM

@philberty
Copy link
Member

some merge conflicts need fixed here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philberty philberty philberty approved these changes

Assignees

@sakupan102 sakupan102

Labels
merge conflicts
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sakupan102 @philberty