Skip to content

Conversation

@WardBrian
Copy link
Member

Follow on to #1004. This tries to incorporate one of @nhuurre's suggestions there to handle array assignment and literals using the Promote expression. I'm thinking Tuples will need something like this as well, if we want to assign (int,int) to (real, int) for example.

I'm not 100% confident this will work as-is (mainly due to my lack of familiarity with the C++ at hand), so I am making this draft to run the tests.

Submission Checklist

  • Run unit tests
  • Documentation
    • If a user-facing facing change was made, the documentation PR is here:
    • OR, no user-facing changes were made

Release notes

Refactor how arrays have their types promoted internally.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

@WardBrian
Copy link
Member Author

@rok-cesnovar this requires more work than I thought, ignore the review and we can revisit after 2.29

@WardBrian WardBrian marked this pull request as ready for review February 2, 2022 20:19
@WardBrian
Copy link
Member Author

This should be ready for review. A brief summary of changes:

  1. Both array expressions and assignment now use the Promotion expression during typechecking.
  2. The code formerly in SignatureMismatch for promotion logic has been moved to its own Promotion module
  3. In code generation, assignment assumes that C++ will do one level of promotion so it strips away one layer to avoid doing duplicate work (e.g., double x = 1; is valid C++, so double x = promote_scalar<double>(1); is just silly)
  4. Complex scalars were changed to use = instead of assign (I think this was an oversight before? If there's a reason for it it was not clear to me, as the changed code still compiles)
  5. The special-cased code for handling array expressions in codegen was removed, it is all done with Promotion now.
  6. I tried my best to remove promotion expressions from things like the partial evaluator -- if we can evaluate it in OCaml directly, we don't need to keep the Promotion information around.

@WardBrian
Copy link
Member Author

@rok-cesnovar Would you mind taking a look at this now that the release is out? Thanks!

@WardBrian WardBrian merged commit 17f651a into master Feb 17, 2022
@WardBrian WardBrian deleted the assignment-promotion branch February 17, 2022 14:51
@WardBrian WardBrian mentioned this pull request Feb 23, 2022
2 tasks
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.

3 participants