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

checker: disallow indexing generic struct receivers #19949

Closed
wants to merge 4 commits into from

Conversation

Delta456
Copy link
Member

@Delta456 Delta456 commented Nov 20, 2023

Fixes #18707

🤖[deprecated] Generated by Copilot at 5ce04ba

Add a check and a test case for invalid index expressions on generic types. This fixes a compiler error or a runtime panic when trying to access a generic type by index that is not a map, array, fixed array, or gated type.

🤖[deprecated] Generated by Copilot at 5ce04ba

  • Add a check for invalid index expressions on generic types that are not maps, arrays, fixed arrays, or gated types (link)
  • Add a test case and source code for the error message that is expected when using an invalid index expression on a generic heap struct (link, link)

@Delta456 Delta456 marked this pull request as draft November 20, 2023 14:05
@Delta456 Delta456 marked this pull request as ready for review November 20, 2023 17:53
s := typ_sym.info as ast.Struct
if s.is_generic && typ.has_flag(.generic) && !node.is_map && !node.is_array
&& !node.is_farray && !node.is_gated {
c.error('cannot index a generic', node.pos)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
c.error('cannot index a generic', node.pos)
c.error('cannot index a generic struct', node.pos)

@spytheman spytheman changed the title checker: disallow indexing generics checker: disallow indexing generic struct receivers Nov 21, 2023
Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

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

imho, adding a test like this, will also be good:

struct Number[T] {
	value T
}

fn f[T](numbers &Number[T], idx int) T {
	return unsafe { numbers[idx].value }
}

fn test_indexing_a_pointer_to_generic_instances() {
	numbers := [10]Number[int]{init: Number[int]{
		value: index * 10
	}}
	assert f(&numbers[0], 3) == 30
	assert f(&numbers[0], 9) == 90
}

It is a useful thing to have, and it compiles and runs on master, but fails with the current implementation of this PR.

@spytheman
Copy link
Member

Closing in favor of #19949 .
Thanks @Delta456 for noticing it 🙇🏻‍♂️ . I have missed that the check there is more general.

@spytheman spytheman closed this Nov 29, 2023
@Delta456 Delta456 deleted the checker_invalid_index_expr branch November 29, 2023 11:27
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.

C error: cannot convert 'int' to 'struct main__Heap_T_int'
2 participants