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

Move specification of deprecated headers to Annex D #1054

Merged
merged 1 commit into from Mar 16, 2017

Conversation

tkoeppe
Copy link
Contributor

@tkoeppe tkoeppe commented Nov 16, 2016

No description provided.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 16, 2016

@AlisdairM, what do you think of this change?

\indextext{library!C standard}%
additional headers, as shown in Table~\ref{tab:cpp.c.headers}.%
The facilities of the C standard library\indextext{library!C standard}
are provided in 22 additional headers, as shown in Table~\ref{tab:cpp.c.headers}.%
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we were trying to avoid keeping accurate counts, as they are too much effort to maintain? Prefer just 'additional headers' without the 22?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we have a blanket rule for this. In the present case, it was easy enough to update the number, and we've previously updated (but kept) counts of things.

Copy link
Contributor

Choose a reason for hiding this comment

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

Happy to go with whatever you decide, just calling it out so that we make a deliberate decision, rather than accidentally fall into the old pattern by inertia.

@@ -1295,7 +1284,7 @@
\ref{support.rtti} & Type identification & \tcode{<typeinfo>} \\ \rowsep
\ref{support.exception} & Exception handling & \tcode{<exception>} \\ \rowsep
\ref{support.initlist} & Initializer lists & \tcode{<initializer_list>} \\ \rowsep
\ref{support.runtime} & Other runtime support & \tcode{<cstdalign>} \tcode{<cstdarg>} \tcode{<cstdbool>} \\ \rowsep
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we can editorially remove these headers from a free-standing implementation - that should require LWG signoff (which I expect should be non-contentious, but I am frequently surprised)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. We can perhaps keep the headers in the free-standing implementation by adding a suitable sentence to Annex D?

Copy link
Contributor

Choose a reason for hiding this comment

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

If everything else is now in Annex D, I would suggest the "add this to freestanding" text belongs in Annex D as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I "fixed" this problem by reverting the changes to Clause 17. I noticed that we already list <strstream> as part of the library, even though we only ever mention it again in Annex D. We can do the same with these four headers.

@@ -134,6 +135,71 @@
the namespace \tcode{std}.
\end{example}

\rSec2[ccomplex.syn]{Header \tcode{<ccomplex>} synopsis}
Copy link
Contributor

Choose a reason for hiding this comment

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

The moved sections should be of the form [depr.stable.name], so [depr.ccomplex.syn] here, and similar changes below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

\indexlibrary{\idxhdr{stdalign.h}}%
The contents of the header \tcode{<cstdalign>} are the same as the C
standard library header \tcode{<stdalign.h>}, with the following changes:
The header \tcode{<cstdalign>} and the header \tcode{<stdalign.h>} shall not
Copy link
Contributor

Choose a reason for hiding this comment

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

My biggest concern is providing the specification for the C header in the deprecated C++ header section, and similarly for stdbool.h. It is also deprecated to use any of the C headers by name in C++, but it still makes me more nervous about this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then we have a conundrum: both headers <cstdbool> and <stdbool.h> are deprecated, yet you want the specification to be in the main text? So the specification is not deprecated, only the use is?

Copy link
Contributor

Choose a reason for hiding this comment

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

As I said, this was the one migration from the main text that gave me most concern - even though both uses are deprecated for different reasons. I' would be for this patch to proceed unless we have a better idea though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a high-level answer to the concern: If the C headers are "deprecated" in the sense of "do not use this", then in the context of compatibility that can only be understood to mean "don't use C". And with that reading, I find it acceptable to define deprecated headers in Annex D, even from scratch.

Note that we do the same for <strstream>.

@tkoeppe tkoeppe added the lwg Issue must be reviewed by LWG. label Nov 16, 2016
@tkoeppe tkoeppe force-pushed the master branch 2 times, most recently from d2a10b4 to c1f9b30 Compare November 17, 2016 14:36
\ref{support.exception} & Exception handling & \tcode{<exception>} \\ \rowsep
\ref{support.initlist} & Initializer lists & \tcode{<initializer_list>} \\ \rowsep
\ref{support.runtime} & Other runtime support & \tcode{<cstdarg>} \\ \rowsep
\ref{depr.cstdalign.syn}, \ref{depr.cstdbool.syn} & Deprecated headers & \tcode{<cstdalign>} \tcode{<cstdbool>} \\ \rowsep
Copy link
Contributor

@AlisdairM AlisdairM Nov 18, 2016

Choose a reason for hiding this comment

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

This row should probably drop to last, to preserve section-ordering in the table.
Other than this, I have no other reason to not land. Nice cleanup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done. Thanks!

@@ -51,6 +51,15 @@
\rSec1[depr.c.headers]{C standard library headers}

\pnum
For compatibility with the C standard library\indextext{library!C standard},
the \Cpp standard library provides headers
\tcode{<ccomplex>}~(\ref{depr.ccomplex.syn}),
Copy link
Member

Choose a reason for hiding this comment

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

We're still in a section "C standard library headers" here, yet we keep talking about C++ headers.
Either rename this section to "Standard library headers" or add a section "C++ standard library headers" that contains the C++-only content. (I'm unsure what remains in the existing "C headers" section in the latter case.)

Copy link
Contributor Author

@tkoeppe tkoeppe Dec 1, 2016

Choose a reason for hiding this comment

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

@jensmaurer: OK, snark alert: If you squint at it right, you could think of "C standard library headers" to be those headers in the standard library that start with "C". Then it would kind of make sense.

Ultimately, everything we describe in this document is about C++. The name "C header" is just a mnemonic. (We cannot and do not specify the C language here.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jensmaurer: How about "Compatibility headers"?

Copy link
Member

Choose a reason for hiding this comment

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

@tkoeppe: The entire clause talks about compatibility features.
Maybe just say "Headers"? And <ccomplex> and friends aren't really for compatibility with C, despite your edit saying so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm. "Headers" sounds oddly bland, but maybe that's the only accurate description of this part...

@jensmaurer
Copy link
Member

The general approach of moving the definitions of the deprecated headers into annex D is good. Annex C is about incompatibilities with C, so we can be non-normative and terse there, with plenty of \refs.

Copy link
Contributor Author

@tkoeppe tkoeppe left a comment

Choose a reason for hiding this comment

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

See also #1054 for a similar issue.

@@ -51,6 +51,15 @@
\rSec1[depr.c.headers]{C standard library headers}

\pnum
For compatibility with the C standard library\indextext{library!C standard},
the \Cpp standard library provides headers
\tcode{<ccomplex>}~(\ref{depr.ccomplex.syn}),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jensmaurer: How about "Compatibility headers"?

@jensmaurer
Copy link
Member

You need to rebase.

@jensmaurer
Copy link
Member

This interacts with LWG 2835 "LWG 2536 seems to misspecify <tgmath.h>".

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Dec 15, 2016

Yes, absolutely.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Feb 4, 2017

LWG agrees with this direction; I'll make a few tweaks and then ask for a review.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Feb 5, 2017

I decided to add a new section "C++ standard library headers [depr.cpp.headers]", as a sibling section to the existing "C standard library headers [depr.c.headers]", and the new section comes first so that the existing one can refer to it.

@zygoloid: Ready for review and application.

@mclow: Could you take a final look to see if this is in accord with LWG guidance?

@mclow
Copy link
Contributor

mclow commented Feb 5, 2017

This looks good to me.

@jensmaurer jensmaurer removed the lwg Issue must be reviewed by LWG. label Feb 7, 2017
@zygoloid zygoloid added this to the C++17 milestone Mar 3, 2017
@tkoeppe tkoeppe force-pushed the cnew branch 2 times, most recently from f464c00 to 826e03a Compare March 16, 2017 01:53
@zygoloid zygoloid merged commit 8139f2f into cplusplus:master Mar 16, 2017
@tkoeppe tkoeppe deleted the cnew branch March 16, 2017 02:15
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.

None yet

5 participants