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

[cstdint, cinttypes.syn] Reorganize <cstdint> presentation #843

Closed
wants to merge 1 commit into from

Conversation

tkoeppe
Copy link
Contributor

@tkoeppe tkoeppe commented Jul 21, 2016

  • Demote heading "18.4.1 Header <cstdint> synopsis" to \synopsis.
  • Show MIN/MAX macros in synopsis.
  • Remove stray namespace comment.
  • Add See Also for the relevant C clause.
  • Add cross reference from <cinttypes> synopsis.

#define UINT@\placeholdernc{N}@_C(value) @\seebelow@

#define INTMAX_C(value) @\seebelow@
#define UINTMAX_C(value) @\seebelow@
\end{codeblock}

\pnum
The header defines all types and macros the same as
Copy link
Member

Choose a reason for hiding this comment

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

I think this has changed the meaning, but I'm not sure whether the old or new wording matches the intent. We used to definitively say that the C++ header definitely defines these macros for N in 8, 16, 32, 64, and in no other cases. C says something quite different -- those macros are defined if there happen to be types of those widths, and other macros can also be defined if there are types of other widths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does C++ not actually allow other values of N at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zygoloid: updated version pushed that lists the four sizes that we support explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zygoloid: Ping


\pnum
The header also defines numerous macros of the form:
#define INT[8, 16, 32, 64]_MIN @\seebelow@ // optional
Copy link
Member

Choose a reason for hiding this comment

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

These did not appear to be optional before.

I do not think we can fix this editorially; we need to ask LWG what they meant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These macros are supposed to expand to expressions of their corresponding type. Since the corresponding type is optional, surely the macro must also be optional?

But indeed the LEAST/FAST macros should absolutely not be optional, that's a mistake that I will fix at once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

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 will open an LWG issue to have the question settled of whether the macros are optional.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 18, 2016

Pending resolution of LWG 2764 and LWG 2820.

@@ -14984,7 +14984,7 @@
\indexlibrary{\idxcode{SCNuPTR}}%
\indexlibrary{\idxcode{SCNxPTR}}%
\begin{codeblock}
#include <cstdint>
#include <cstdint> // see \ref{cstdint}

Copy link
Member

Choose a reason for hiding this comment

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

Is that comment aligned to one of your preferred columns?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me check. Preferred columns are 20, 32, 48, 24.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@@ -1306,7 +1306,7 @@

\rSec1[cstdint]{Integer types}

\rSec2[cstdint.syn]{Header \tcode{<cstdint>} synopsis}
\synopsis{Header \tcode{<cstdint>} synopsis}
\indextext{\idxhdr{cstdint}}%
Copy link
Member

Choose a reason for hiding this comment

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

\synopsis is gone; see #566 and #1231.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll rebase 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.

Rebased, not sure how to avoid the pointless substructure.

\end{codeblock}

\pnum
The header defines all types and macros the same as
the C standard library header \tcode{<stdint.h>}.

\xref ISO C~7.20.

\rSec1[support.start.term]{Start and termination}
Copy link
Member

Choose a reason for hiding this comment

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

This seems a good addition that can go in ahead of time.

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 in edd2c5f.

@tkoeppe tkoeppe force-pushed the inttypes branch 4 times, most recently from 5c10b40 to 9155f47 Compare December 15, 2016 01:32
using int8_t = @\textit{signed integer type}@; // optional
using int16_t = @\textit{signed integer type}@; // optional
using int32_t = @\textit{signed integer type}@; // optional
using int64_t = @\textit{signed integer type}@; // optional

Copy link
Member

Choose a reason for hiding this comment

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

Could you please put in the whitespace fixes right away? This makes the "hard" parts easier to spot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverting those changes for now, since they don't take italic correction into account, either. Once we have direction, I'll separate cosmetic and substantive change.


plus function macros of the form:
#define INT_FAST[8, 16, 32, 64]_MIN @\seebelow@
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems worse: the never-defined pattern-matching language of "INT_[FAST LEAST]{8 16 32 64}_MIN" is both changed and used in what would otherwise be a preprocessor directive (rather than prose).

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Apr 2, 2018

I think this is pending some LWG issue (LWG 2820). I'll check with @mclow where we are on this.

@jensmaurer
Copy link
Member

@tkoeppe, ping. It seems LWG 2820 is ready for a -lib vote and will make some of the changes in this pull request moot. I'd suggest you ask @mclow.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Mar 15, 2019

Any day now, seriously.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Dec 14, 2020

@mclow, @jensmaurer: This has been resolved by LWG2820 via 2020-11 Motion LWG-3. Thank you all for your continued support!

@tkoeppe tkoeppe closed this Dec 14, 2020
@tkoeppe tkoeppe deleted the inttypes branch December 14, 2020 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lwg Issue must be reviewed by LWG.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants