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

Translate _d_newarray{mTX, miTX, Op} to a single template #15819

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

teodutu
Copy link
Member

@teodutu teodutu commented Nov 16, 2023

  • Move code for _d_newarraymTX to core.internal.array.construction
  • Remove _d_newarraymiTX and _d_newarraymOp
  • Add unittests for _d_newarraymTX
  • Move lowering to _d_newarraymTX to the semantic phase
  • Inline the lowering when inlining NewExps
  • Add changelog entry about the new hook

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @teodutu!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#15819"

@teodutu teodutu force-pushed the template-_d_newarraymT branch 2 times, most recently from 6884ad9 to a6e64d3 Compare November 16, 2023 00:25
@teodutu teodutu changed the title wip: Translate _d_newarray{mTX, miTX, Op} to a single template Translate _d_newarray{mTX, miTX, Op} to a single template Nov 16, 2023
@teodutu teodutu marked this pull request as ready for review November 16, 2023 00:26
@teodutu
Copy link
Member Author

teodutu commented Nov 16, 2023

I'll improve the hook and its nested function in the coming days. Up to now, I just wanted something that works and hopefully passes the CI tests.

@teodutu teodutu force-pushed the template-_d_newarraymT branch 2 times, most recently from 84975ff to 78e2c8f Compare November 16, 2023 06:34
Copy link
Contributor

@RazvanN7 RazvanN7 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@RazvanN7
Copy link
Contributor

RazvanN7 commented Nov 16, 2023

Looks like some profile gc tests need to be updated.

./generated/linux/debug/64/test_cycles --DRT-oncycle=print > generated/linux/debug/64/cycle_print.done 2>&1; test $? -eq 0
3,4c3,4
<             160	              1	float[][] D main src/profilegc.d:18
<             160	              1	int[][] D main src/profilegc.d:15
---
>             160	              1	float D main src/profilegc.d:18
>             160	              1	int D main src/profilegc.d:15

@teodutu teodutu force-pushed the template-_d_newarraymT branch 3 times, most recently from 78bb38c to 90dfb5b Compare November 16, 2023 17:09
@RazvanN7
Copy link
Contributor

@teodutu maybe also add a changelog entry to be consistent with the previously templated hook?

@teodutu teodutu force-pushed the template-_d_newarraymT branch 2 times, most recently from 693f694 to c6122bf Compare November 16, 2023 21:35
- Move code for `_d_newarraymTX` to `core.internal.array.construction`
- Remove `_d_newarraymiTX` and `_d_newarraymOp`
- Add unittests for `_d_newarraymTX`
- Move lowering to `_d_newarraymTX` to the semantic phase
- Inline the lowering when inlining `NewExp`s
- Add changelog entry about the new hook

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
@teodutu
Copy link
Member Author

teodutu commented Nov 16, 2023

Done @RazvanN7.

@RazvanN7 RazvanN7 merged commit 055accf into dlang:master Nov 17, 2023
43 checks passed
@tim-dlang
Copy link
Contributor

This caused a regression: https://issues.dlang.org/show_bug.cgi?id=24436

Since #16097 the regression does not happen any more for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
4 participants