On Sat, 22 Jul 2023 04:44:21 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> See https://bugs.java.com/bugdatabase/view_bug?bug_id=8305734 and >> https://bugs.java.com/bugdatabase/view_bug?bug_id=JDK-8311905 > > Hi, thanks for the background and the discussion of the alternatives. I'm not > sure that drafting the CSR is the right next step; there are a number of > foibles about editing the specifications that come into play when preparing > the CSR. However, what you've written above is a good design discussion about > the different alternatives and how they affect the specification and the > implementation. > > One thing that we need to consider is compatibility. From an excessively > pedantic standpoint, any change in behavior is a potential incompatibility > (somebody might be relying on that bug!) but I think that fixing obviously > incorrect behavior is reasonable. As much as possible I'd like to make sure > that things that worked, at least partially, continue to work. > > With this in mind I'm leaning toward allowing a BitSet to contain bits > including the Integer.MAX_VALUE bit, and adjusting various specs accordingly. > I think the best way to specify this is to say that length() returns an int > in the range [0, 2^31] as an unsigned value. In practice of course this means > it can return any Java `int` value in the range [0, MAX_VALUE] and also > Integer.MIN_VALUE. A sufficiently careful programmer can use such values > correctly, as long as they avoid doing certain things such as comparing > against zero. (We might want to have a note in the spec to that effect.) > > It's good that you analyzed the `valueOf` methods. It looks to me like the > implementation will store an actual array potentially containing bits beyond > the MAX_VALUE bit, and this will affect things like length() and size() and > such bits won't be accessible via get() or set(). So, on the one hand, this > behavior seems clearly broken and ought to be fixed by limiting the input > array along the lines suggested by your three options. > > On the other hand, it seems that from looking at the code, it's possible to > create an "over-size" BitSet with valueOf(), and perform bulk bit > manipulations on it and other BitSets using methods like and(), or(), and > xor(). It also appears possible to read out the bits successfully using > toByteArray() or toLongArray(). Thus an application might be able to > manipulate bit arrays of up to about 2^37 bits long by using BitSet with long > arrays or LongBuffers. Restricting the input of valueOf() to 2^31 bits would > break such applications. > > Also note that the specification says the bits are numbered by nonnegative > integers (that is, zero to 2^31) which would seem to preclude longer bit > arrays. However, if somebody... Hey @stuart-marks (cc @liach @AlanBateman @ExE-Boss), I think this discussion has stalled a bit. Would it be an idea to at least push this commit through, so the get(i,j) bug at least is fixed, and then maybe move the larger discussion to a different place? (Eg. a pull request for [JDK-8311905](https://bugs.openjdk.org/browse/JDK-8311905)?) I don't believe the CSR is _essential_ to fix the get(i,j) issue, and clearly there is some disagreement on what the right solution is. If we do end up changing the spec for the BitSet, there will almost certainly be larger changes in the class than just the get(i,j) method. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13388#issuecomment-1782673878
