Add support for compiler-defined union initializers#28984
Open
bradcray wants to merge 9 commits into
Open
Conversation
Where compiler-generated initializers for classes and records have an argument per field, for unions, it makes more sense to have an initializer per field since only one field can be active at a time. Prior to this PR, we only had a compiler-generated 0-argument initializer, which is also useful for putting the union into its uninitialized state. This PR adds an initializer per field, accepting a single argument of that field type. A new test is added to check that the initializers work; and an old test is updated to reflect the new overloads. This was fairly simple due to the fact that unions currently don't support type-less fields, fields with initializers, type fields, or param fields. If/when those things change, it could become more complex. One such change that feels reasonable would be for a single union field to support an initializer, at which point the 0-argument initializer we currently generate could be replaced by the initializer for that field taking a default argument. Future work TODO: - [ ] open an issue for the one-field initializer idea above --- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
--- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
--- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
…should've I was accidentally using the `this` argument from the 0-argument union initializer for _part of_ the 1-argument union initializers, which caused things to blow up in a big way when the 0-argument initializer wasn't called. Basically, I believe it would get dead-code eliminated, taking the _this with it, and breaking other routines. It was one of the weirdest behaviors I've ever seen, though, where valgrind didn't complain and we tried calling qualType() on a former ArgSymbol that the compiler considered to now be a BaseAST, so we got a C++ "pure virtual method called" error. --- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
… post-parse checks Made an INT_FATAL into a USR_FATAL; added new tests locking in the errors (seemingly we didn't have any Mystery: Why do the post-process checks not catch typeless multi-var decls --- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
--- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
Rationale: DEFAULT here apparently means "can be called with 0 arguments" rather than "is created by the compiler as a default since the user didn't supply any." --- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
--- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
…ed union inits This fixes union-init3.chpl --- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Where compiler-generated initializers for classes and records have an argument per field, for unions, it makes more sense to have an initializer per field since only one field can be active at a time. Prior to this PR, we only had a compiler-generated 0-argument initializer, which is also useful for putting the union into its uninitialized state. This PR adds an initializer per field, accepting a single argument of that field type. A new test is added to check that the initializers work; and an old test is updated to reflect the new overloads.
This was fairly simple due to the fact that unions currently don't support type-less fields, fields with initializers, type fields, or param fields. If/when those things change, it could become more complex. One such change that feels reasonable would be for a single union field to support an initializer, at which point the 0-argument initializer we currently generate could be replaced by the initializer for that field taking a default argument. Future work
TODO:
test/classes/diten/constructUnion.chplfails unless it contains anew u()call; and similarly my test fails if I comment out that line…Resolves #4972 (once item 3 above is done)