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

[dcl.enum] CWG 1636: Fix calculation for bmax #1349

Closed
wants to merge 1 commit into from

Conversation

hyrosen
Copy link

@hyrosen hyrosen commented Jan 9, 2017

This addresses CWG 1636

Section 7.2 [dcl.enum], describes a calculation for determining the legal range of values that a variable of enumeration type can hold, and the minimum number of bits needed to hold a value of that type. That calculation is based on the smallest and largest literal values, emin and emax, and is wrong for certain cases. It currently says:

Otherwise, for an enumeration where emin is the smallest enumerator and emax is the largest, the values of the enumeration are the values in the range bmin to bmax, defined as follows: Let K be 1 for a two’s complement representation and 0 for a ones’ complement or sign-magnitude representation. bmax is the smallest value greater than or equal to max(|emin| − K, |emax|) and equal to 2**M − 1, where M is a non-negative integer. bmin is zero if emin is non-negative and −(bmax + K) otherwise. The size of the smallest bit-field large enough to hold all the values of the enumeration type is max(M, 1) if bmin is zero and M + 1 otherwise.

Here is an example where the Standard is wrong.

Suppose we have enum { N = -1, Z = 0 } on a 2's-complement machine. Following the standard's calculation steps above, we have
emin = -1, emax = 0, and K = 1, so
max(|emin| - K, |emax|) = max(|-1| - 1, |0|) = max(0, 0) = 0,
giving bmax = 0 == 2**M - 1, so M == 0.
Thus M + 1 == 1 bit is needed, and bmin = -(bmax + K) == -(0 + 1) == -1, so the range of legal values for the enumeration is [-1 .. 0]. Seems right.

On the other hand, suppose we have enum { N = -1 }. We expect the same results. 0 is always legal for an enumeration value, so the absence of a 0-valued literal should make no difference. But we do not get the same results. Here
emin = -1, emax = -1 and K = 1, so
max(|emin| − K, |emax|) == max(|-1| - 1, |-1|) == max(0, 1) == 1,
giving bmax = 1 == 2**M - 1, so now M == 1.
Thus M + 1 == 2 bits are needed, and bmin = -(bmax + K) == -(1 + 1) == -2, so the range of
legal values for the enumeration is [-2 .. 1]. This cannot possibly be correct. By removing a literal with value 0, we have gone from requiring 1 bit to 2, and have doubled the range of legal values!

My proposed fix is to replace |emax| with emax. With this change, the enum { N = -1 } case gives
emin = -1, emax = -1 and K = 1, so
max(|emin| − K, emax) == max(|-1| - 1, -1) == max(0, -1) == 0,
giving bmax = 0 == 2**M - 1, so M == 0 and all is well again. We match the enum { N = -1, Z = 0 } case.

We now want to explore what happens if we make this change. For many cases, nothing changes at all.

  1. If emax >= 0, nothing changes since then emax == |emax|.
    Therefore in what follows, we take it that emin <= emax < 0.
  2. On a 1's-complement system, K is defined to be 0, as per the Standard above.
    old: max(|emin| − K, |emax|) == max(|emin|, |emax|) == |emin|
    new: max(|emin| − K, emax) == max(|emin|, emax) == |emin|
    and again nothing changes.
    Therefore in what follows, we assume 2's-complement and have K defined to be 1.
  3. Since emin <= emax < 0, we have
    max(|emin| − K, emax) == max(|emin| − 1, emax) == |emin| − 1
    If this value is to represent a change from the current standard, it must be that
    max(|emin| − K, |emax|) == max(|emin| − 1, |emax|) == |emax|
    otherwise both the old and new values would be the same. Therefore
    |emin| − 1 < |emax|, i.e., |emin| <= |emax|.
    With both negative, this inequality means emin >= emax. But we also have emin <= emax (from Add a GNU Makefile to simplify building the PDF #1), so this means that a change happens only for emin == emax.
    Therefore, in what follows, we assume that we are on a 2's-complement system dealing with an enumeration that has a single literal defined, whose value is a negative integer. Let's designate that value as -N. Then
    old: max(|emin| − K, |emax|) == max(N - 1, N) == N
    new: max(|emin| − K, emax) == max(N − 1, -N) == N - 1
    Now, the standard defines bmax to be the smallest value greater than or equal to this value that is of the form 2M - 1 where M >= 0. Therefore, the old and new calculations for bmax differ only when N - 1 is of the form 2M - 1, i.e., N is a power of 2.

So finally we see that the change affects only enumerations containing a single literal whose value is the negative of a power of 2, when on a 2's-complement system. (There may actually be multiple literals, but they must all have the same value.) For those enumerations, the change means that one fewer bit is needed.

Given that emin == emax == -N == -(2**M), the standard will now say that bmax == N - 1 and then bmin == -(bmax + K) == -N. M bits are sufficient for enumeration values of this type, with values in the range [-N .. N - 1].

@Eelis
Copy link
Contributor

Eelis commented Jan 10, 2017

Needs rebase (due to #1339).

@jensmaurer
Copy link
Member

This is an active core issue involving a normative change. So, this is probably not editorial.

@jensmaurer
Copy link
Member

Sent to the core reflector asking for review: http://lists.isocpp.org/core/2017/01/1470.php

@jensmaurer jensmaurer added the cwg Issue must be reviewed by CWG. label Jan 10, 2017
@jensmaurer jensmaurer added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Feb 17, 2018
@jensmaurer jensmaurer changed the title Fix calculation for bmax. [dcl.enum] Fix calculation for bmax Feb 17, 2018
@tkoeppe
Copy link
Contributor

tkoeppe commented Apr 2, 2018

@jensmaurer: Is there a CWG issue for this?

@jensmaurer
Copy link
Member

This is CWG1636.

@tkoeppe
Copy link
Contributor

tkoeppe commented Apr 3, 2018

@jensmaurer, @zygoloid: Do we need to keep this editorial issue open if this is already tracked as a CWG issue?

@jensmaurer
Copy link
Member

jensmaurer commented Apr 3, 2018

@tkoeppe, since we're on the verge of limiting ourselves to two's complement representation for signed integers (see P0907 Signed Integers are Two’s Complement), I'd actually like to see the fix in that paper. In any case, we should make progress here one way or another.

@jensmaurer jensmaurer changed the title [dcl.enum] Fix calculation for bmax [dcl.enum] CWG 1636: Fix calculation for bmax Apr 12, 2018
@zygoloid zygoloid force-pushed the master branch 2 times, most recently from e3dbfe2 to 1a21a65 Compare July 7, 2018 23:19
@jensmaurer
Copy link
Member

Closing, since this covered by CWG 1636.

@jensmaurer jensmaurer closed this Nov 8, 2018
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

4 participants