> Found while working on JDK-8369699.
> 
> In general, the Vector API documentation is a bit vague around part numbers.
> There was considerable confusion around expansion/contraction: are we talking 
> about logical, physical or output expansion/contraction? Confusingly, in some 
> places we called expansions things that go from more to fewer bits, and we 
> called contractions that went from fewer to more bits.
> 
> And exception messages are not very helpful, for example they don't provide 
> the legal range.
> 
> @rose00 took a first stab at improving things 
> (https://github.com/openjdk/jdk/pull/29306), and I eventually took over the 
> project.
> 
> --------------------------------------
> 
> Principles:
> - Expansion means fewer->more bits.
> - Contraction means more->fewer bits.
> - Be clear about input, logical result and output.
> - We primarily use:
>   - conversion lanewise expansion (logical)
>   - conversion lanewise contraction (logical)
>   - conversion lanewise in-place (logical)
>   - reinterpret (logical)
>   - select, for truncation (output)
>   - insert, for padding (output)
>   - in-place, logical fits output (output)
> 
> Please review this PR in this order:
> - Changes in the "Expansions, contractions, and partial results" section of 
> `Vector.java`. We must first agree on the definitions here, before we go and 
> disagree elsewhere ;)
> - Changes in affected methods `convertShape`, `convert`, and 
> `reinterpretShape`.
> - Internal changes in `AbstractVector.java‎`: adjust nomenclature and 
> exception message.
> - New test. I think it is necessary, I caught some mistakes I made. And when 
> I wanted to add tests for `unslice` I realized that it does not throw for out 
> of bounds `part`. So I think it is justified.
> 
> In general, I'm a bit worried that the documentation is a bit too long, and 
> feels a bit heavy/overwhelming.
> To a large degree, this is due to the complexity of part numbers.
> We could drop some paragraphs and some repetition. Let me know what you think 
> is too much.
> More explanations may help make things clearer, but also risk being too much 
> and overwhelming.
> I'm open to cut things down more, and any other constructive suggestions ;)
> 
> -----------------------
> 
> While reading the documentation and testing for `unslice`, I found out that 
> https://github.com/openjdk/jdk/pull/3804 accidentally removed the bounds 
> checks for `part`. But we did not notice, because there were no tests for it. 
> I'm adding the bounds check back in and adding tests for it as well.
> 
> ---------
> - [x] I confirm that I make this contribution in ac...

Emanuel Peter has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains 46 additional commits 
since the last revision:

 - Merge branch 'master' into 
JDK-8375631-part-number-exception-and-documentation
 - fix resut->result typo for John
 - after merge with master: fix fuzzer for unslice out of bounds exception
 - Merge branch 'master' into 
JDK-8375631-part-number-exception-and-documentation
 - small update for Paul
 - more for Paul
 - fix reinterpret examples in table, suggested by Paul
 - Apply suggestions from code review
   
   Co-authored-by: Paul Sandoz <[email protected]>
 - fix small typo
 - update and restructure resizing list
 - ... and 36 more: https://git.openjdk.org/jdk/compare/389f230f...7c5d35cf

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/30113/files
  - new: https://git.openjdk.org/jdk/pull/30113/files/5ed014cd..7c5d35cf

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=30113&range=13
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30113&range=12-13

  Stats: 13875 lines in 706 files changed: 9219 ins; 2117 del; 2539 mod
  Patch: https://git.openjdk.org/jdk/pull/30113.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/30113/head:pull/30113

PR: https://git.openjdk.org/jdk/pull/30113

Reply via email to