Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Motions 2023 02 lwg 13] P2679R2 Fixing std::start_lifetime_as and std::start_lifetime_as_array #6114

Merged
merged 1 commit into from Mar 11, 2023

Conversation

RobertLeahy
Copy link
Contributor

@RobertLeahy RobertLeahy commented Feb 14, 2023

@frederick-vs-ja
Copy link
Contributor

This also fixes cplusplus/papers#1345.

@tkoeppe
Copy link
Contributor

tkoeppe commented Feb 14, 2023

Thank you!

@@ -927,15 +928,33 @@
\end{itemdecl}

\begin{itemdescr}
\pnum
\mandates
\tcode{T} is a complete type.
Copy link
Member

Choose a reason for hiding this comment

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

I know this is what the approved paper says, but is there any reason that the non-array form requires "not an incomplete type" and the array form requires "is a complete type"? It seems like they could both say it the same way.

Copy link
Member

Choose a reason for hiding this comment

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

I had the same thoughts during the initial review but never send a comment about this ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing a quick search through the draft I see both forms used elsewhere in the standard. I'm unsure if the distinction was deliberate since I don't precisely remember the details of the completeness discussion in Core in Kona.

Interestingly "T is a complete type" doesn't seem to come with a cross reference elsewhere whereas "T is not an incomplete type" does seem to (to term.incomplete.type) which I'll add to this PR in the next revision.

Copy link
Contributor

Choose a reason for hiding this comment

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

We're a bit inconsistent in the library. Core folk will tell you that we don't define the term "complete type", we only define "incomplete type". Library insists that we can use "complete type" with the obvious English-language meaning "a type which is not an incomplete type", so you will sometimes see "complete type" and sometimes "not an incomplete type".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it therefore fine for this change to be internally inconsistent (as specified in the paper) or can/should we editorially coalesce around "complete type" or "not an incomplete type" consistently?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to have LWG come back with some guidance, and we'll take the paper as approved for now. But it's a good question worth settling, @jwakely.

source/memory.tex Show resolved Hide resolved
@burblebee
Copy link
Contributor

burblebee commented Feb 17, 2023

Hey guys, I signed up for this motion at #6103. My commit is in motions-2023-02-lwg-13.

Please follow our process - you need to sign up for the github issue before taking the motion to avoid duplicate work. I didn't notice this "fix" for the issue when I signed up for it. Alas.

source/memory.tex Show resolved Hide resolved
source/memory.tex Outdated Show resolved Hide resolved
@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 11, 2023

@burblebee Very sorry about that, we'll make this clearer next time!

@tkoeppe tkoeppe requested a review from jwakely March 11, 2023 19:58
@tkoeppe tkoeppe dismissed jwakely’s stale review March 11, 2023 19:58

Out of scope, please advise with LWG guidance in separate issue/PR.

@tkoeppe tkoeppe merged commit 767455c into cplusplus:main Mar 11, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants