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

[basic.fundamental] Provide more information about which types are integral #3974

Closed
wants to merge 1 commit into from

Conversation

sdkrystian
Copy link
Contributor

Saying "standard and extended integer types" provides more information to the reader than "signed and unsigned integer types" (integer type implies signed and unsigned) and doesn't alter meaning.

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

I don't think this is an improvement. Highlighting standard/extended is no different than highlighting signed/unsigned.

@sdkrystian
Copy link
Contributor Author

@jensmaurer It's certainly a trivial change, but I see the question of "are extended integer types integral types?"/"Do the standard conversions apply to extended integer types?" more often than "do the standard integer types include unsigned and signed standard integer types" (not seen this one yet, luckly :P).

Specifying standard/extended subtly provides more information to the reader in the context of that single paragraph, which I believe is useful as paragraphs in the standard are typically not read sequentially.

@zygoloid
Copy link
Member

My first reaction was that this change is incorrect: we should just define that "integer types" are "standard and extended integer types". But it appears that bool, char, wchar_t, and charN_t aren't actually "standard integer types". That seems like a normative error to me. Consider the (very few) uses of "standard integer type":

[conv.rank]/1.5: "The rank of any standard integer type shall be greater than the rank of any extended integer type with the same width."
[conv.rank]/1.7: "The rank of bool shall be less than the rank of all other standard integer types."

These both obviously meant to include all non-extended integer types.

[utility.intcmp]/1, /5, /9: "Mandates: Both T and U are standard integer types or extended integer types (6.8.2)."

I have no idea what the intent was here.

This needs CWG and LWG attention.

@zygoloid zygoloid added cwg Issue must be reviewed by CWG. lwg Issue must be reviewed by LWG. labels Sep 18, 2020
@jensmaurer
Copy link
Member

jensmaurer commented Sep 20, 2020

@zygoloid , I think we can remove this from CWG scope, because we have

conv.rank p1.8: "The ranks of char8_t, char16_t, char32_t, and wchar_t shall equal the ranks of their underlying types"
and we have transitivity via p1.10, so those are covered.

We should fix "[conv.rank]/1.7: "The rank of bool shall be less than the rank of all other standard integer types."
to avoid the hint that "bool" is a standard integer type.

Admittedly, that leaves open the possibility that bool has greater rank than an extended integer type, but that feels like design intent.

Regarding "[utility.intcmp]/1, /5, /9: "Mandates: Both T and U are standard integer types or extended integer types (6.8.2)."", there is a note at the bottom that clarifies the intent:

[Note 1 : These function templates cannot be used to compare byte, char, char8_t, char16_t, char32_t, wchar_t, and bool. — end note]

So, except for "other" in conv.rank, I think this works as intended and we should simply close this pull request.

@jwakely
Copy link
Member

jwakely commented Sep 20, 2020

We chose the wording in [utility.intcmp] specifically because it doesn't include bool, char etc.

The wording matches the intent.

@wg21bot wg21bot added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Jun 15, 2021
@jensmaurer jensmaurer closed this Dec 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cwg Issue must be reviewed by CWG. needs rebase The pull request needs a git rebase to resolve merge conflicts.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants