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

lower field access in the presence of a vptr #4515

Closed

Conversation

dwblaikie
Copy link
Contributor

@dwblaikie dwblaikie commented Nov 12, 2024

Add one to the field index when lowering, if there's a vptr that needs to be skipped over.

@dwblaikie dwblaikie mentioned this pull request Nov 12, 2024
@dwblaikie dwblaikie closed this Nov 19, 2024
@dwblaikie dwblaikie deleted the lower_vptr_field_access branch November 19, 2024 22:51
github-merge-queue bot pushed a commit that referenced this pull request Nov 26, 2024
We considered a couple of other options for this:
* #4515 Keep the
`ElementIndex` numbering vptr-ignorant, and do +1 offsets as needed -
seems subtle/easy to miss
* #4517 Always have a
zeroth element in the object representation, make it zero-size in the
case of no-vptr - @zygoloid was concerned this would add overhead
especially to stateless objects used in type-trait-like things.

But currently moving forward with this direction - of initializing field
indexes with an invalid value until the end of the class definition,
then assigning field indexes during construction of the class's object
representation struct type. This direction might reinforce/help avoid
premature access to the object representation before the class is
complete, and give a single place where class layout is done (at class
completion) if we want to add more options there, such as class layout
optimizations, etc.

This patch still has problems with object initialization (that #4515
does not have/does address) but does address normal `obj.member` access
correctly.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
bricknerb pushed a commit to bricknerb/carbon-lang that referenced this pull request Nov 28, 2024
We considered a couple of other options for this:
* carbon-language#4515 Keep the
`ElementIndex` numbering vptr-ignorant, and do +1 offsets as needed -
seems subtle/easy to miss
* carbon-language#4517 Always have a
zeroth element in the object representation, make it zero-size in the
case of no-vptr - @zygoloid was concerned this would add overhead
especially to stateless objects used in type-trait-like things.

But currently moving forward with this direction - of initializing field
indexes with an invalid value until the end of the class definition,
then assigning field indexes during construction of the class's object
representation struct type. This direction might reinforce/help avoid
premature access to the object representation before the class is
complete, and give a single place where class layout is done (at class
completion) if we want to add more options there, such as class layout
optimizations, etc.

This patch still has problems with object initialization (that carbon-language#4515
does not have/does address) but does address normal `obj.member` access
correctly.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant