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

fix(cpp1): account for initializers of members lowered as bases #478

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented May 30, 2023

Resolves #422.

Testing summary.
100% tests passed, 0 tests failed out of 704

Total Test time (real) = 104.69 sec
Acknowledgements.

@JohelEGP JohelEGP force-pushed the default_constructor-this_initializer branch 2 times, most recently from 707d3b0 to 5c72314 Compare May 31, 2023 01:11
@JohelEGP JohelEGP changed the title fix(cpp1): account for initializers of this members fix(cpp1): account for initializers of this members May 31, 2023
@JohelEGP JohelEGP force-pushed the default_constructor-this_initializer branch from 5eafcee to 431a3a0 Compare June 7, 2023 15:40
@JohelEGP JohelEGP changed the title fix(cpp1): account for initializers of this members fix(cpp1): account for initializers of members lowered as bases Jun 7, 2023
@JohelEGP JohelEGP force-pushed the default_constructor-this_initializer branch 2 times, most recently from 5eafcee to 9d05e45 Compare June 7, 2023 15:46
@JohelEGP JohelEGP force-pushed the default_constructor-this_initializer branch from 9d05e45 to 7c3ebe2 Compare June 16, 2023 19:13
@JohelEGP
Copy link
Contributor Author

JohelEGP commented Jun 17, 2023

With commit 379ae03,
a @struct lowers to an aggregate.
If an @struct type also has members lowered as bases,
it's necessary to continue emitting those SMFs.
-- #422 (comment)

@JohelEGP JohelEGP marked this pull request as draft June 17, 2023 18:39
@JohelEGP
Copy link
Contributor Author

I suppose the only reasonable option is to queue an error if emitting SMFs is suppressed.

I can't reliably lower an initializer to the *_as_base's data member's initializer,
as it could only be valid in the derived type's scope
(e.g., uses the type's template parameter or a previous data member).

I thought about also emitting actual bases to *_as_struct_base,
but besides being equally problematic to move the lowered initializer,
the change from a direct base class to an indirect one should come with its own set of problems.

I wonder if the error falls under the banner of "(temporary alpha limitation)".

@JohelEGP JohelEGP force-pushed the default_constructor-this_initializer branch from 7c3ebe2 to c5a42ef Compare June 18, 2023 16:39
@JohelEGP JohelEGP marked this pull request as ready for review June 18, 2023 16:39
@JohelEGP JohelEGP force-pushed the default_constructor-this_initializer branch 6 times, most recently from a300079 to 30c64ca Compare June 19, 2023 20:42
source/parse.h Outdated Show resolved Hide resolved
source/reflect.h2 Show resolved Hide resolved
source/reflect.h2 Show resolved Hide resolved
@JohelEGP JohelEGP force-pushed the default_constructor-this_initializer branch from 30c64ca to 9977b9a Compare July 23, 2023 19:48
@hsutter
Copy link
Owner

hsutter commented Oct 31, 2024

Hi! Sorry it took me so long to get to this one... this one looks like it might be out of date, should I close it for now and you can reopen this or create a new PR when it's ready? Again, my apologies for not keeping up.

@hsutter hsutter added the question - further information requested Further information is requested label Oct 31, 2024
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.

[BUG] Generated default constructor ignores this's initializer
2 participants