Skip to content

Update copy constructor section#3554

Closed
jcouv wants to merge 2 commits intodotnet:masterfrom
jcouv:copy-ctor
Closed

Update copy constructor section#3554
jcouv wants to merge 2 commits intodotnet:masterfrom
jcouv:copy-ctor

Conversation

@jcouv
Copy link
Member

@jcouv jcouv commented Jun 10, 2020

No description provided.

@jcouv
Copy link
Member Author

jcouv commented Jun 10, 2020

@dotnet/roslyn-compiler for review (spec change).


* A protected constructor taking a single argument of the record type.
* A constructor taking a single argument of the record type, referred to as the "copy constructor"
* A public parameterless virtual instance "clone" method with a compiler-reserved name
Copy link
Member

Choose a reason for hiding this comment

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

clone [](start = 43, length = 5)

This should probably be italics instead of quoted.

fields of `this`.
#### Copy constructor

If a user-defined copy constructor is present, it is used instead of a synthesized one. It must use
Copy link
Member

@gafter gafter Jun 10, 2020

Choose a reason for hiding this comment

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

present [](start = 38, length = 7)

Not sure what "present" means. Is this intended to overrule the text from the enclosing section "Members are synthesized unless an accessible concrete (non-abstract) member with a "matching" signature is either inherited or declared in the record body." or is it intended to be a restatement? If a restatement, I suggest "declared". #Resolved

Copy link
Member Author

@jcouv jcouv Jun 10, 2020

Choose a reason for hiding this comment

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

Thanks. Removed the unnecessary restatement, keeping the new portion. #Resolved

fields from the record itself:
1. If the record derives from `System.Object`, the copy constructor invokes the default constructor on the base type.
Otherwise, it invokes the copy constructor on the direct base record, passing its argument through. An error is
produced if that base copy constructor is not accessible.
Copy link
Member

Choose a reason for hiding this comment

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

. [](start = 56, length = 1)

These options are not exhaustive. What if it extends a non-record type?

Copy link
Member Author

Choose a reason for hiding this comment

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

We only allow deriving from object or a record at the moment. Further LDM discussion may change that.

Copy link
Member

Choose a reason for hiding this comment

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

There is no such constraint in this specification. Please either add that constraint or fix this section to account for its absence.


In reply to: 438205208 [](ancestors = 438205208)

Otherwise, it invokes the copy constructor on the direct base record, passing its argument through. An error is
produced if that base copy constructor is not accessible.
2. The copy constructor then copies the values of all the instance fields in the input record to the corresponding
fields of `this`, considering only the fields declared by the record.
Copy link
Member

@gafter gafter Jun 10, 2020

Choose a reason for hiding this comment

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

declared [](start = 46, length = 8)

"declared" is used as the opposite of "synthesized" elsewhere. Should this be "synthesized or declared"? #Resolved

Copy link
Member Author

@jcouv jcouv Jun 10, 2020

Choose a reason for hiding this comment

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

Fixed. I'm mainly trying to oppose to inherited fields (those are handled by the base copy constructor). #Resolved

@AlekseyTs
Copy link
Contributor

It looks like some of the similar changes were already made in an earlier PR

fields of `this`.
#### Copy constructor

If a user-defined copy constructor is present, it must use the base's copy constructor as its
Copy link
Member

Choose a reason for hiding this comment

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

present [](start = 38, length = 7)

Still think this should be "declared".

@jcouv jcouv closed this Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants