Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

[WIP] Fixes needed for the _d_arrayappend{T,cTX} dmd PR#2718

Closed
Vild wants to merge 3 commits intodlang:masterfrom
Vild:ArrayAppendRelatedFixes
Closed

[WIP] Fixes needed for the _d_arrayappend{T,cTX} dmd PR#2718
Vild wants to merge 3 commits intodlang:masterfrom
Vild:ArrayAppendRelatedFixes

Conversation

@Vild
Copy link
Contributor

@Vild Vild commented Aug 3, 2019

No description provided.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @Vild! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

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 fetch digger
dub run digger -- build "master + druntime#2718"

Vild added 3 commits August 14, 2019 23:07
Signed-off-by: Dan Printzell <xwildn00bx@gmail.com>
Signed-off-by: Dan Printzell <xwildn00bx@gmail.com>
Signed-off-by: Dan Printzell <xwildn00bx@gmail.com>
@Vild Vild force-pushed the ArrayAppendRelatedFixes branch from 1b79661 to 0ec45b3 Compare August 14, 2019 21:08
@@ -139,7 +147,7 @@ template _d_arrayappendTImpl(Tarr : T[], T)
* Run postblit on `t` if it is a struct and needs it.
* Or if `t` is a array, run it on the children if they have a postblit.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

If the return type is changing, I recommend updating the DDoc comment to include a Returns explanation.

* if the postblit is callable in a `pure` scope, if it exist.
* if it does not exist, return true.
*/
template isPostblitPure(T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Curly braces on new line

__traits(isSame, T, __traits(parent, T.init.__xpostblit)))
enum isPostblitPure = isPure!(T.init.__xpostblit);
else
enum isPostblitNoThrow = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

isPostblitPure instead of isPostblitNoThrow?

t.__xpostblit();
{
import core.internal.array.utils : isPure;
static if (isPure!(t.__xpostblit))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be static if (isPostblitPure!T)? I don't see isPostblitPure used anywhere.

auto sizeelem = T.sizeof;

_d_arrayappendcTXImpl!Tarr._d_arrayappendcTX(x, y.length);
if (_d_arrayappendcTXImpl!Tarr._d_arrayappendcTX(x, y.length) is null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Under what condition would _d_arrayappendcTxImpl return null?

@Vild Vild changed the title Fixes needed for the _d_arrayappend{T,cTX} dmd PR [WIP] Fixes needed for the _d_arrayappend{T,cTX} dmd PR Aug 30, 2019
@dlang-bot dlang-bot added the WIP Work In Progress - not ready for review or pulling label Aug 30, 2019
@dlang-bot dlang-bot added Needs Rebase needs a `git rebase` performed stalled labels May 27, 2021
@RazvanN7 RazvanN7 closed this Dec 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Needs Rebase needs a `git rebase` performed stalled WIP Work In Progress - not ready for review or pulling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants