Skip to content

Translate _d_newitem{U,T,iT} to a template - #14664

Merged
RazvanN7 merged 1 commit into
dlang:masterfrom
teodutu:template-_d_newitemT
May 12, 2023
Merged

Translate _d_newitem{U,T,iT} to a template#14664
RazvanN7 merged 1 commit into
dlang:masterfrom
teodutu:template-_d_newitemT

Conversation

@teodutu

@teodutu teodutu commented Dec 2, 2022

Copy link
Copy Markdown
Member

This contributes to dlang/project-ideas#25 and makes the following changes:

  • Replace _d_newitem{U,T,iT} with a template _d_newitemT that allocates memory for the new struct and performs the required initialisation
  • Move lowering call to _d_newitemT from e2ir.d to expressionsem.d
  • Add lowering field to NewExp that contains the lowered call and use it in e2ir.d
  • Remove _d_newitem{,i}T from rt.lifetime.d

@teodutu
teodutu requested a review from ibuclaw as a code owner December 2, 2022 01:10
@dlang-bot

dlang-bot commented Dec 2, 2022

Copy link
Copy Markdown

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#14664"

@UplinkCoder

UplinkCoder commented Dec 2, 2022

Copy link
Copy Markdown
Member

tests seem to be failing.

@teodutu
teodutu force-pushed the template-_d_newitemT branch from 97942db to 2d9def4 Compare December 2, 2022 04:08
@teodutu

teodutu commented Dec 2, 2022

Copy link
Copy Markdown
Member Author

Yeah @UplinkCoder, uniformctor.d fails because __traits(isZeroInit) seems to be true for enums. I'm not sure why implicit.d fails yet.

teodutu
teodutu commented Dec 2, 2022
View reviewed changes
Comment thread druntime/test/profile/bothgc.log.exp Outdated
Comment thread druntime/test/profile/myprofilegc.log.linux.64.exp Outdated
@RazvanN7 RazvanN7 mentioned this pull request Dec 2, 2022
Merged
@teodutu
teodutu force-pushed the template-_d_newitemT branch from 2d9def4 to 934d4a3 Compare January 23, 2023 14:06
@teodutu

teodutu commented Jan 23, 2023

Copy link
Copy Markdown
Member Author

runnable/implicit.d is failing because of this bug: https://issues.dlang.org/show_bug.cgi?id=23650.

@RazvanN7 RazvanN7 mentioned this pull request Jan 24, 2023
Merged
@teodutu
teodutu force-pushed the template-_d_newitemT branch from 934d4a3 to c175c64 Compare April 24, 2023 10:58
@teodutu

teodutu commented Apr 24, 2023

Copy link
Copy Markdown
Member Author

I managed to avoid https://issues.dlang.org/show_bug.cgi?id=23650 by avoiding the lowering altogether inside __traits(compiles).

@teodutu
teodutu force-pushed the template-_d_newitemT branch 9 times, most recently from d3f3c67 to 1d245f5 Compare April 26, 2023 11:05
@kinke kinke mentioned this pull request May 7, 2023
Merged
@PetarKirov

PetarKirov commented May 7, 2023

Copy link
Copy Markdown
Member

@teodutu please rebase

@teodutu
teodutu force-pushed the template-_d_newitemT branch 2 times, most recently from eea4973 to 8ccce9e Compare May 8, 2023 04:04
@teodutu

teodutu commented May 8, 2023

Copy link
Copy Markdown
Member Author

Done @PetarKirov. I'll look into the traceGC errors later this week.

@teodutu
teodutu force-pushed the template-_d_newitemT branch 2 times, most recently from 206e85b to 5400302 Compare May 10, 2023 08:19
RazvanN7
RazvanN7 reviewed May 10, 2023
View reviewed changes
Comment thread compiler/src/dmd/expressionsem.d Outdated
@teodutu
teodutu force-pushed the template-_d_newitemT branch from 5400302 to fedf8e5 Compare May 10, 2023 09:16
@teodutu
teodutu requested a review from RazvanN7 May 10, 2023 09:16
@RazvanN7

RazvanN7 commented May 10, 2023

Copy link
Copy Markdown
Contributor

cc @dkorpel @PetarKirov this seems to be green now.

@dkorpel dkorpel mentioned this pull request May 10, 2023
Closed
@RazvanN7 RazvanN7 added the Merge:72h no objection -> merge The PR will be merged if there are no objections raised. label May 10, 2023
dkorpel
dkorpel requested changes May 10, 2023
View reviewed changes

@dkorpel dkorpel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mostly style nits

Comment thread druntime/src/core/lifetime.d Outdated
Comment thread druntime/src/core/lifetime.d Outdated
Comment thread druntime/src/core/lifetime.d Outdated
Comment thread druntime/src/core/lifetime.d Outdated
Comment thread druntime/src/core/lifetime.d Outdated
Comment thread druntime/src/core/lifetime.d Outdated
Comment thread druntime/src/core/lifetime.d Outdated
Comment thread druntime/src/core/lifetime.d Outdated
Comment thread compiler/src/dmd/expressionsem.d
Comment thread compiler/src/dmd/expressionsem.d Outdated
@teodutu
teodutu force-pushed the template-_d_newitemT branch from fedf8e5 to 7077de1 Compare May 11, 2023 05:21
@teodutu

teodutu commented May 11, 2023

Copy link
Copy Markdown
Member Author

Done @dkorpel. Take another look when you can.

@teodutu
teodutu force-pushed the template-_d_newitemT branch 3 times, most recently from 207c374 to 1e21a7c Compare May 11, 2023 05:37
@teodutu
Translate _d_newitem{U,T,iT} to a single template
960326c 
This makes the following changes:
- Replace `_d_newitem{U,T,iT}` with a template `_d_newitemT` that
allocates memory for the new `struct` and performs the required
initialisation
- Move lowering call to `_d_newitemT` from e2ir.d to expressionsem.d
- Add `lowering` field to `NewExp` that contains the lowered call and
use it in e2ir.d
- Remove `_d_newitem{,i}T` from rt.lifetime

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
@teodutu
teodutu force-pushed the template-_d_newitemT branch from 1e21a7c to 960326c Compare May 12, 2023 06:47
dkorpel
dkorpel approved these changes May 12, 2023
View reviewed changes

@dkorpel dkorpel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought you were going to forward Unqual!T in druntime, but unqualifying in the compiler works too.

@RazvanN7
RazvanN7 merged commit 159a880 into dlang:master May 12, 2023
nybzmr added a commit to nybzmr/dmd that referenced this pull request Mar 19, 2025
@nybzmr
Remove RTLSYM for Translation PR dlang#14664: Removed NEWITEMT, NEWIT…
6bcbfa7 
…EMIT, TRACENEWITEMT and TRACENEWITEMIT
thewilsonator pushed a commit that referenced this pull request Mar 19, 2025
@nybzmr
Remove Unused RTLSYMs from DMD (#21032)
cfefcdd 
* Remove RTLSYM for Translation PR #15819: Removed NEWARRAYMITX, NEWARRAYMITX, TRACENEWARRAYMTX and TRACENEWARRAYMITX

* Remove RTLSYM for Translation PR #15299: Removed NEWARRAYT, NEWARRAYIT, TRACENEWARRAYT and TRACENEWARRAYIT

* Remove RTLSYM for Translation PR #14837: Removed NEWCLASS, TRACENEWCLASS

* Remove RTLSYM for Translation PR #14664: Removed NEWITEMT, NEWITEMIT, TRACENEWITEMT and TRACENEWITEMIT

* Remove RTLSYM for Translation PR #14550: Removed ARRAYCATNTX, ARRAYCATT, TRACEARRAYCATNTX and TRACEARRAYCATT

* Remove RTLSYM for Translation PR #14382: Removed ARRAYSETASSIGN

* Remove RTLSYM for Translation PR #14310: Removed ARRAYASSIGN

* Remove RTLSYM for Translation PR #13495: Removed ARRAYAPPENDT, ARRAYAPPENDCTX, TRACEARRAYAPPENDT and TRACEARRAYAPPENDCTX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dkorpel dkorpel dkorpel approved these changes
@RazvanN7 RazvanN7 RazvanN7 approved these changes
@ibuclaw ibuclaw Awaiting requested review from ibuclaw ibuclaw is a code owner

Assignees

No one assigned

Labels

Merge:72h no objection -> merge The PR will be merged if there are no objections raised.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@teodutu @dlang-bot @UplinkCoder @PetarKirov @RazvanN7 @dkorpel