Skip to content

Fix Issue 8006: Property Binary Assignment Operators (Spec Change)#1865

Closed
JinShil wants to merge 3 commits intodlang:masterfrom
JinShil:master
Closed

Fix Issue 8006: Property Binary Assignment Operators (Spec Change)#1865
JinShil wants to merge 3 commits intodlang:masterfrom
JinShil:master

Conversation

@JinShil
Copy link
Contributor

@JinShil JinShil commented Aug 14, 2017

Spec change to accompany dlang/dmd#7079

FYI: This does not address unary assignment operators (e.g. s.x++, ++s.x). I intend to implement that in a future pull request.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @JinShil! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close Bugzilla Description
8006 Implement proper in-place-modification for properties

spec/function.dd Outdated
$(LI $(D @property) functions can only have zero, one or two parameters.)
$(LI $(D @property) functions cannot have variadic parameters.)
$(LI Binary assignment operators can be used if the appropriate getter and setter $(D @property) functions
exist.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you should clarify what the semantics are (e.g. by mentioning equivalent code).
"appropriate getter and setter", is too vague to my taste (this is The Spec after all).

Copy link
Contributor

Choose a reason for hiding this comment

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

This also needs something like: Binary assignment can be used when the getter returns by ref. If the getter does not return by ref, and a setter is defined, binary assignment is equivalent with setter(getter() @ value), where @ is the operation.

Copy link
Contributor

Choose a reason for hiding this comment

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

(somewhere you should mention (and test in the other PR) the interplay with opBinary overloading)

Copy link
Contributor Author

@JinShil JinShil Aug 17, 2017

Choose a reason for hiding this comment

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

somewhere you should mention the interplay with opBinary overloading

I'm assuming you mean the opOpAssign overloads. Please correct me if I'm wrong.

The only ambiguity I see with the proposed implementation is when there is an alias thised property and a binary assignment operator overload (i.e. opOpAssign).

As an illustration of how things work today, consider the following code where there is an alias thised property x, and the assignment operator overload opAssign.

struct TInt
{
    int mX;

    @property int x() { return mX; }
    @property void x(int v) { mX = v; }

    int mB;
    void opAssign(int v) { mB = v; }

    alias x this;
}

void main()
{
    TInt t;
    t.x = 4;
    t = 5;    // Which do you think should be called? `x(5)` or `opAssign(5)`
	
    assert(t.mX == 4);
    assert(t.mB == 5);
}

This behavior is already defined in the spec at https://dlang.org/spec/class.html#alias-this

If the member is a class or struct, undefined lookups will be forwarded to the AliasThis member.

Since opAssign is not undefined t = 5 will call t.opAssign(5), and the binary assignment operators (i.e. opOpAssign) should work in the same way.

Since this behavior is already defined in the spec, I propose we don't duplicate it here in the Property Functions section.

and test in the other PR

I will be adding tests to the DMD PR to check that the implementation conforms to this specified behavior. Thank you for bringing it to my attention.

spec/function.dd Outdated
Foo f;

f.data = 3; // same as f.data(3);
f.data += 3; // same as f.data(fdata() + 3);
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: forgot the dot.

Copy link
Contributor

Choose a reason for hiding this comment

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

The semantic interpretation of f.data+=3 ==> f.data(f.data()+3) is not true if the getter returns by ref. So this needs to be specified and clarified.

spec/function.dd Outdated
$(LI Binary assignments can be used with $(D @property) functions. If the getter does not return by `ref`,
the return value is not an aggregate that overloads the binary assignment operator,
and a setter is defined, binary assignment is equivalent to `setter(getter() @ value)`, where `@` is the
operation.)
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned in dlang/dmd#7079, it should be specified whether the object is evaluated once or twice and whether it's copied or not (should be evaluated once and never copied). At best the spec would define the operation through a lowering to simpler constructs.

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

LGTM as a start (pending addressing Andrei's comment)

@wilzbach
Copy link
Contributor

See also: #1872

@JinShil JinShil closed this Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants