dmd
dmd copied to clipboard
WIP: Move last occurrence of symbols when occurring in rhs of VarExpr or if passed to a function declaration [skip ci]
This enable automatic move of last occurrence of an l-value struct instance in a VarExp when occurring
- as a right-hand-side
VarExpof anAssignExp - as a value-passed
Parameter - as the single
VarExprof aReturnStatement - ...
It took me and @UplinkCoder about 2-3 hours online to accomplish this. Superb cooperation.
Hopefully this will make https://github.com/dlang/phobos/pull/8537 unnecessary.
C++23 is going in that direction aswell - see https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2266r3.html.
TODOs:
- [ ] Discuss if it's ok to add
VarExp lastVarExpandbool lastVarExpCanBeMovedto VarDeclaration or if should go with a much smaller number of cases that can be handled typicallyreturn VarExp- ... = VarExp; being the last statement in the scope of
VarExp
- [ ] Check for move in
return VarExp_after the correspondingVarDeclarationhas been resolved. Useasm { int 3; }where VarDeclaration is resolved and continue in gdb. - [ ] Handle
ReturnStatementcase as well. If scope ofReturnStatementmatches that ofVariableDeclarationofVarExpmove is unconditional. Cases to handle are:- [ ]
return XwhereXisVarExp - [ ]
return f(Xs...)for allXinXsbeing aVarExpr - [ ]
return P ? X : YAll these cases are easiest handled with an extra state in visitor keeping track of currentReturnStatement(if any).
- [ ]
- [ ] Should move structs with disabled copy constructor (
moveOnDisabledPostblitin added test filemovetest.dshould pass) - [ ] Test that copy constructors are correctly avoided aswell in assignments
- [ ] Test that structs with
@disable this(this)are allowed in the three cases outline above - [ ] Handle if-statements and other branching structures.
- [ ] Limit moves to when scope of
VarDeclarationequals scope ofVarExp. - [ ] Check if
opPostMoveneeds to be taken into consideration here - [ ] Regenerate C++ headers
- [ ] Update tests now that compiler optimizes avoid away more copy constructor and destructor calls and potentially inserts calls to
opPostMove. - [ ] Adjust TODO comments added
- [ ] Discuss if an alternatie solution avoiding extra state in
VarDeclarationin favor of extra state inSemanticTimeVisitoris preferred. - [ ] Close https://github.com/dlang/phobos/pull/8537 as doesn't need fix
- [ ] Add warning diagnostics about unnecessary calls to
core.lifetime.move - [ ] Remove all other calls to
move()that are no longer neccessary.
Current simple test
make -f posix.mak; generated/linux/release/64/dmd -vcg-ast -c ~/movetest.d
where movetest.d is
struct S
{
int x;
~this() @safe pure nothrow @nogc
{
import std.stdio;
debug writeln(__FUNCTION__);
}
}
void moveOn(S s) @safe pure nothrow @nogc
{
S t = s; // s is moved here and t i
}
void moveOff(S s) @safe pure nothrow @nogc
{
S t = s; // is not moved here because
s.x = 42; // it's reference here
}
correctly outputs
import object;
struct S
{
int x;
~this()
{
}
alias __xdtor = ~this()
{
}
;
pure nothrow @nogc ref @trusted S opAssign(S p) return
{
(S __swap2 = void;) , __swap2 = this , (this = p , __swap2.~this());
return this;
}
}
pure nothrow @nogc @safe void moveOn(S s)
{
S t = s;
t.~this();
}
pure nothrow @nogc @safe void moveOff(S s)
{
S t = s;
s.x = 42;
t.~this();
s.~this();
}
RTInfo!(S)
{
enum immutable(void)* RTInfo = null;
}
NoPointersBitmapPayload!1LU
{
enum ulong[1] NoPointersBitmapPayload = [0LU];
}
FYI, @maxhaton @atilaneves @andralex @WalterBright.
Thanks for your pull request and interest in making D better, @nordlow! 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:andReturns:)
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 run digger -- build "master + dmd#14377"
Since there's been no progress on this, and the [skip ci] indicates that there's no need to see the CI output, I'll close this for now