Fix #20583 - Initialisation of @mustuse member variable causes spur…

[thewilsonator]

…ious error

[dlang-bot]

Thanks for your pull request and interest in making D better, @thewilsonator! 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.

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. Please add the word "Bugzilla" to issue references. For example, Fix Bugzilla Issue 12345 or Fix Bugzilla 12345.(Reminder: the edit needs to be done in the Git commit message, not the GitHub pull request.)

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

[pbackus]

Looking at ExpressionSemanticVisitor.visit(AssignExp), there are several places where this kind of rewrite is done, and all of them are slightly different. The relevant code is here:

dmd/compiler/src/dmd/expressionsem.d

Lines 10808 to 11079 in 32a6dfe

	if (exp.op == EXP.construct)
	{
	Type t2 = e2x.type.toBasetype();
	if (t2.ty == Tstruct && sd == (cast(TypeStruct)t2).sym)
	{
	sd.size(exp.loc);
	if (sd.sizeok != Sizeok.done)
	return setError();
	if (!sd.ctor)
	sd.ctor = sd.searchCtor();
	// https://issues.dlang.org/show_bug.cgi?id=15661
	// Look for the form from last of comma chain.
	auto e2y = lastComma(e2x);
	CallExp ce = (e2y.op == EXP.call) ? cast(CallExp)e2y : null;
	DotVarExp dve = (ce && ce.e1.op == EXP.dotVariable)
	? cast(DotVarExp)ce.e1 : null;
	if (sd.ctor && ce && dve && dve.var.isCtorDeclaration() &&
	// https://issues.dlang.org/show_bug.cgi?id=19389
	dve.e1.op != EXP.dotVariable &&
	e2y.type.implicitConvTo(t1))
	{
	/* Look for form of constructor call which is:
	* __ctmp.ctor(arguments...)
	*/
	/* Before calling the constructor, initialize
	* variable with a bit copy of the default
	* initializer
	*/
	Expression einit = getInitExp(sd, exp.loc, sc, t1);
	if (einit.op == EXP.error)
	{
	result = einit;
	return;
	}
	auto ae = new BlitExp(exp.loc, exp.e1, einit);
	ae.type = e1x.type;
	/* Replace __ctmp being constructed with e1.
	* We need to copy constructor call expression,
	* because it may be used in other place.
	*/
	auto dvx = cast(DotVarExp)dve.copy();
	dvx.e1 = e1x;
	auto cx = cast(CallExp)ce.copy();
	cx.e1 = dvx;
	if (checkConstructorEscape(*sc, cx, false))
	return setError();
	Expression e0;
	Expression.extractLast(e2x, e0);
	auto e = Expression.combine(e0, ae, cx);
	e = e.expressionSemantic(sc);
	result = e;
	return;
	}
	// https://issues.dlang.org/show_bug.cgi?id=21586
	// Rewrite CondExp or e1 will miss direct construction, e.g.
	// e1 = a ? S(1) : ...; -> AST: e1 = a ? (S(0)).this(1) : ...;
	// a temporary created and an extra destructor call.
	// AST will be rewritten to:
	// a ? e1 = 0, e1.this(1) : ...; -> blitting plus construction
	if (e2x.op == EXP.question)
	{
	/* Rewrite as:
	* a ? e1 = b : e1 = c;
	*/
	CondExp econd = cast(CondExp)e2x;
	Expression ea1 = new ConstructExp(econd.e1.loc, e1x, econd.e1);
	Expression ea2 = new ConstructExp(econd.e2.loc, e1x, econd.e2);
	Expression e = new CondExp(exp.loc, econd.econd, ea1, ea2);
	result = e.expressionSemantic(sc);
	return;
	}
	if (sd.postblit || sd.hasCopyCtor)
	{
	/* We have a copy constructor for this
	*/
	//printf("exp: %s\n", toChars(exp));
	//printf("e2x: %s\n", toChars(e2x));
	if (e2x.isLvalue())
	{
	if (sd.hasCopyCtor)
	{
	/* Rewrite as:
	* e1 = init, e1.copyCtor(e2);
	*/
	Expression einit = new BlitExp(exp.loc, exp.e1, getInitExp(sd, exp.loc, sc, t1));
	einit.type = e1x.type;
	Expression e;
	e = new DotIdExp(exp.loc, e1x, Id.ctor);
	e = new CallExp(exp.loc, e, e2x);
	e = new CommaExp(exp.loc, einit, e);
	//printf("e: %s\n", e.toChars());
	result = e.expressionSemantic(sc);
	return;
	}
	else
	{
	if (!e2x.type.implicitConvTo(e1x.type))
	{
	error(exp.loc, "conversion error from `%s` to `%s`",
	e2x.type.toChars(), e1x.type.toChars());
	return setError();
	}
	/* Rewrite as:
	* (e1 = e2).postblit();
	*
	* Blit assignment e1 = e2 returns a reference to the original e1,
	* then call the postblit on it.
	*/
	Expression e = e1x.copy();
	e.type = e.type.mutableOf();
	if (e.type.isShared && !sd.type.isShared)
	e.type = e.type.unSharedOf();
	e = new BlitExp(exp.loc, e, e2x);
	e = new DotVarExp(exp.loc, e, sd.postblit, false);
	e = new CallExp(exp.loc, e);
	result = e.expressionSemantic(sc);
	return;
	}
	}
	else if (sd.hasMoveCtor && !e2x.isCallExp() && !e2x.isStructLiteralExp())
	{
	/* The !e2x.isCallExp() is because it is already an rvalue
	and the move constructor is unnecessary:
	struct S {
	alias TT this;
	long TT();
	this(T)(int x) {}
	this(S);
	this(ref S);
	~this();
	}
	S fun(ref S arg);
	void test() { S st; fun(st); }
	*/
	/* Rewrite as:
	* e1 = init, e1.moveCtor(e2);
	*/
	Expression einit = new BlitExp(exp.loc, exp.e1, getInitExp(sd, exp.loc, sc, t1));
	einit.type = e1x.type;
	Expression e;
	e = new DotIdExp(exp.loc, e1x, Id.ctor);
	e = new CallExp(exp.loc, e, e2x);
	e = new CommaExp(exp.loc, einit, e);
	//printf("e: %s\n", e.toChars());
	result = e.expressionSemantic(sc);
	return;
	}
	else
	{
	/* The struct value returned from the function is transferred
	* so should not call the destructor on it.
	*/
	e2x = valueNoDtor(e2x);
	}
	}
	// https://issues.dlang.org/show_bug.cgi?id=19251
	// if e2 cannot be converted to e1.type, maybe there is an alias this
	if (!e2x.implicitConvTo(t1))
	{
	AggregateDeclaration ad2 = isAggregate(e2x.type);
	if (ad2 && ad2.aliasthis && !isRecursiveAliasThis(exp.att2, exp.e2.type))
	{
	/* Rewrite (e1 op e2) as:
	* (e1 op e2.aliasthis)
	*/
	exp.e2 = new DotIdExp(exp.e2.loc, exp.e2, ad2.aliasthis.ident);
	result = exp.expressionSemantic(sc);
	return;
	}
	}
	}
	else if (!e2x.implicitConvTo(t1))
	{
	sd.size(exp.loc);
	if (sd.sizeok != Sizeok.done)
	return setError();
	if (!sd.ctor)
	sd.ctor = sd.searchCtor();
	if (sd.ctor)
	{
	/* Look for implicit constructor call
	* Rewrite as:
	* e1 = init, e1.ctor(e2)
	*/
	/* Fix Issue 5153 : https://issues.dlang.org/show_bug.cgi?id=5153
	* Using `new` to initialize a struct object is a common mistake, but
	* the error message from the compiler is not very helpful in that
	* case. If exp.e2 is a NewExp and the type of new is the same as
	* the type as exp.e1 (struct in this case), then we know for sure
	* that the user wants to instantiate a struct. This is done to avoid
	* issuing an error when the user actually wants to call a constructor
	* which receives a class object.
	*
	* Foo f = new Foo2(0); is a valid expression if Foo has a constructor
	* which receives an instance of a Foo2 class
	*/
	if (exp.e2.op == EXP.new_)
	{
	auto newExp = cast(NewExp)(exp.e2);
	if (newExp.newtype && newExp.newtype == t1)
	{
	error(exp.loc, "cannot implicitly convert expression `%s` of type `%s` to `%s`",
	newExp.toChars(), newExp.type.toChars(), t1.toChars());
	errorSupplemental(exp.loc, "Perhaps remove the `new` keyword?");
	return setError();
	}
	}
	Expression einit = new BlitExp(exp.loc, e1x, getInitExp(sd, exp.loc, sc, t1));
	einit.type = e1x.type;
	Expression e;
	e = new DotIdExp(exp.loc, e1x, Id.ctor);
	e = new CallExp(exp.loc, e, e2x);
	e = new CommaExp(exp.loc, einit, e);
	e = e.expressionSemantic(sc);
	result = e;
	return;
	}
	if (search_function(sd, Id.call))
	{
	/* Look for static opCall
	* https://issues.dlang.org/show_bug.cgi?id=2702
	* Rewrite as:
	* e1 = typeof(e1).opCall(arguments)
	*/
	e2x = typeDotIdExp(e2x.loc, e1x.type, Id.call);
	e2x = new CallExp(exp.loc, e2x, exp.e2);
	e2x = e2x.expressionSemantic(sc);
	e2x = resolveProperties(sc, e2x);
	if (e2x.op == EXP.error)
	{
	result = e2x;
	return;
	}
	if (e2x.checkValue() || e2x.checkSharedAccess(sc))
	return setError();
	}
	}
	else // https://issues.dlang.org/show_bug.cgi?id=11355
	{
	AggregateDeclaration ad2 = isAggregate(e2x.type);
	if (ad2 && ad2.aliasthis && !isRecursiveAliasThis(exp.att2, exp.e2.type))
	{
	/* Rewrite (e1 op e2) as:
	* (e1 op e2.aliasthis)
	*/
	exp.e2 = new DotIdExp(exp.e2.loc, exp.e2, ad2.aliasthis.ident);
	result = exp.expressionSemantic(sc);
	return;
	}
	}
	}

To ensure that we have proper coverage of all these cases, the test should include

• An example that uses a copy constructor.
• An example that uses a move constructor.
• An example that uses a postblit.
• An example where the right-hand side of the assignment is a conditional expression.