-
Notifications
You must be signed in to change notification settings - Fork 61
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
8282953: Drop MemoryLayout::map #667
Conversation
👋 Welcome back mcimadamore! A progress list of the required criteria for merging this PR into |
Webrevs
|
src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java
Outdated
Show resolved
Hide resolved
src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java
Outdated
Show resolved
Hide resolved
@mcimadamore This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
Co-authored-by: Paul Sandoz <paul.d.sandoz@googlemail.com>
Co-authored-by: Paul Sandoz <paul.d.sandoz@googlemail.com>
/integrate |
Going to push as commit f645d4f. |
@mcimadamore Pushed as commit f645d4f. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
This patch removes the
MemoryLayout::map
method. This method was introduced to add some support for VLA (back when we had unbounded sequence layouts).This method is quite limited when it comes to define transformations, as it can only alter one layout per call. As such, it is not useful to define global transformations to e.g. flip the endianness of all value layouts nested in a given layout.
We believe we can do much better than this (and have few ideas floating around) - but for the time being we'd like not to take the "good" name for a method that just doesn't seem all that useful.
When removing the method, I realized that the
LayoutPath
class had a lot of unneeded complexity:addStride
andaddScaledOffset
which did same thing; I dropped the first - the duplication was introduced when we dropped workarounds for "small" segments, in fact the leadingMemorySegment
parameter in the first method is now unused;Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/panama-foreign pull/667/head:pull/667
$ git checkout pull/667
Update a local copy of the PR:
$ git checkout pull/667
$ git pull https://git.openjdk.java.net/panama-foreign pull/667/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 667
View PR using the GUI difftool:
$ git pr show -t 667
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/panama-foreign/pull/667.diff