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

[2018-06 LWG Motion 27] P0619R4 Reviewing deprecated facilities of C++17 for C++20 #2144

Closed
jensmaurer opened this issue Jun 9, 2018 · 10 comments
Assignees
Milestone

Comments

@jensmaurer
Copy link
Member

jensmaurer commented Jun 9, 2018

No description provided.

@jensmaurer jensmaurer added this to the post-2018-06 milestone Jun 9, 2018
@tkoeppe
Copy link
Contributor

tkoeppe commented Jun 9, 2018

Careful with the C-header removals: there are lots of issues with the Annex C and D changes that we need to look at carefully.

Issue:
[diff.mods.to.headers]p2 should be amended to now also list <stdbool.h> and <stdalign.h> and <iso646.h>.

Issue:

But the corresponding Annex C entry says:
"""
A valid C++ 2017 program that includes any of the following headers may fail to compile: , <ciso646>, <cstdalign>, <cstdbool>, and <ctgmath>. The #include directives can simply be removed with no loss of meaning.
"""

I think this is incorrect: only three of the headers are (mostly) vacuous; however, and have non-trivial content (specified in Annex D, e.g. depr.ccomplex.syn). That mans that those header inclusions cannot be removed with loss of meaning. Instead, they should be replaced by the relevant set of non-deprecated headers.

We could also talk about whether losing __alignas_is_defined constitutes "loss of meaning", but I don't much care about that.

Issue:

We've completely deleted the specifications of <stdalign.h> and <stdbool.h>, which are headers that we have and will keep. Their specification was included in that of their corresponding, nonsensical <cname> headers, but we do need to retain that aspect.

@AlisdairM
Copy link
Contributor

I'll take this one if someone can assign it to me?

@AlisdairM
Copy link
Contributor

Another problem in the Annex C wording is the 'Changes' line refers to clauses removed from the document, so we cannot cross-reference. I see two options here:

  1. Simply drop the 'affected clause' marker for those changes (we have precedent)
  2. Where possible, refer to the original clause the deprecated libraries affected,
    such as shared_ptr, allocator, or the functional typedefs.

I lean towards (2), but would like an Editor's opinion before proceeding.

@tkoeppe
Copy link
Contributor

tkoeppe commented Jun 11, 2018

@AlisdairM: Are you preparing a pull request for this, or would you like us to do that?

@zygoloid
Copy link
Member

Assigned to Alisdair per earlier comment. Alisdair, please unassign if you don't want to take this.

For the Annex C entries, I think referring to the clause the deprecated libraries affected is a reasonable approach, but it's not what we did before -- see diff.cpp14.depr for how we handled this for the C++17 removals.

Please follow the pattern of [diff.cpp14.depr] for now (that is: what's in the wording paper, minus the "Affected subclause" headings), and open an issue for us to consider whether [diff.cpp*.depr] should contain cross-references to the non-deprecated portion of the affected library.

AlisdairM added a commit to AlisdairM/draft that referenced this issue Jun 12, 2018
Apply the changes of P0619R4 directly, without patching issues revealed
along the way.  Reflow table 17 to better accomodate 21 headers.

Some known issues to be addressed under cplusplus#2144
@AlisdairM
Copy link
Contributor

I have pushed #2173 but this does /not/ address any of the above points yet. Per Thomas's suggestion at Rapperswil, I am pushing the original paper before making fixes.

If someone else wants to commandeer this branch and complete the editorial changes, I will not be working on it again until tomorrow morning (June 13).

Two additional issues noted applying the paper:
23.12.2 [mem.res.class]
missing default constructor on memory_resource (should be defaulted, is that editorial?)

C.5.7 [diff.cpp17.depr]
No pnum when missing Affected Clause (I /think/ I followed zygoloid's direction correctly here)

zygoloid pushed a commit that referenced this issue Jun 22, 2018
Apply the changes of P0619R4 directly, without patching issues revealed
along the way.  Reflow table 17 to better accomodate 21 headers.

Some known issues to be addressed under #2144
@zygoloid
Copy link
Member

[diff.cpp03.language.support] is a problem. It says "valid C++03 code that replaces global new or delete operators may execute differently in this International Standard". But that's a lie: there is no valid C++03 code that replaces global delete and is also valid C++20 code. In C++03, you must declare operator delete(void*) throw(); in C++20, you must declare operator delete(void*) noexcept. We could use a #if __cplusplus < 201103L or similar, but that feels horrible, and also defeats the point of this wording.

So I think we can actually just delete that section, and maybe add some wording to [diff.cpp17.except] to point out that we have removed the ability to share a single declaration of operator delete(void*) between C++03 and C++20.

@zygoloid
Copy link
Member

And actually, the problem with [diff.cpp03.language.support] isn't even new. There's never been a way to declare a replacement operator new(size_t) that works in both C++98 and C++11, so [diff.cpp03.language.support] has always been wrong.

zygoloid pushed a commit that referenced this issue Jun 23, 2018
Apply the changes of P0619R4 directly, without patching issues revealed
along the way.  Reflow table 17 to better accomodate 21 headers.

Some known issues to be addressed under #2144
@zygoloid
Copy link
Member

@AlisdairM I've merged your branch into our motions-2018-06-lwg-27 branch and made the other changes requested here. The result is now ready for review.

@zygoloid zygoloid assigned zygoloid and unassigned AlisdairM Jun 23, 2018
@zygoloid
Copy link
Member

@AlisdairM

23.12.2 [mem.res.class]
missing default constructor on memory_resource (should be defaulted, is that editorial?)

That changes the normative effect of the wording in a way that contradicts the instructions in the wording paper. Let's use the LWG issue process to fix that.

zygoloid added a commit that referenced this issue Jun 25, 2018
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

No branches or pull requests

4 participants