Skip to content

feat(libSceJson2): implement Array::iterator assignment, postfix increment and Object::Pair - #4476

Closed
ahmedmakroum wants to merge 2 commits into
boykopovar:mainfrom
ahmedmakroum:feat/json2-iterator-pair
Closed

ahmedmakroum wants to merge 2 commits into
boykopovar:mainfrom
ahmedmakroum:feat/json2-iterator-pair

Conversation

@ahmedmakroum

@ahmedmakroum ahmedmakroum commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

Implements five libSceJson2 exports that were NotImplemented stubs:

  • Array::iterator::iterator() (placement new on guest memory)
  • Array::iterator::operator=(const iterator&)
  • Array::iterator::operator++(int) (returns the old position through the hidden result pointer, like Array::begin())
  • Object::Pair::Pair(const String&, const Value&) (deep copies key and value with the existing Construct and CopyInto helpers, sets the reserved field to 0)
  • Object::Pair::~Pair() (destroys key and value)

The other Json2 stubs are left as they are: Object::insert has no return type in its mangled name, and the handler setters and operator bool have unknown behaviour.

Tested

Linux (WSL2), no title. Added IteratorAssignmentAndPair to the guest_json test: default construction followed by assignment, postfix increment returning the old position and advancing the iterator, Pair deep copy (changing and destroying the source String and Value leaves the pair unchanged), and the Pair destructor. ctest -R '^guest_json$' passes and python3 tools/check_conventions.py passes.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • AI-assisted: yes, written with Claude Code and reviewed by me

@ahmedmakroum

Copy link
Copy Markdown
Contributor Author

The overlap bot links #4447. That PR adds parameter checks to these five stubs but keeps them NotImplemented; this PR implements them. They change the same lines, so I’d suggest this one goes first and #4447 drops its changes to these five Json2 exports. I’ve left a note there too.

YousefMohsen0 added a commit to YousefMohsen0/AnyPS5 that referenced this pull request Oct 12, 2026
…and add parameter validation

- libSceJson2: Drop 5 functions overlapping with boykopovar#4476 (Array::iterator ctor/op++/op=, Pair ctor/dtor)
  Fix Value::clear to keep notifyError call
  Fix MemAllocator::notifyError signature (3 params)
  Fix Value callbacks signatures with self parameter
  Fix Object::insert, Array::iterator operators with proper signatures
- libSceBluetoothHid: Add parameter validation to GetDeviceName, GetInputReport, SetOutputReport
- All functions follow project pattern: validation + NotImplemented_nid_no_patch
@oneandonlydean oneandonlydean self-assigned this Oct 12, 2026
YousefMohsen0 added a commit to YousefMohsen0/AnyPS5 that referenced this pull request Oct 12, 2026
…and add parameter validation

- libSceJson2: Drop 5 functions overlapping with boykopovar#4476 (Array::iterator ctor/op++/op=, Pair ctor/dtor)
  Fix Value::clear to keep notifyError call
  Fix MemAllocator::notifyError signature (3 params)
  Fix Value callbacks signatures with self parameter
  Fix Object::insert, Array::iterator operators with proper signatures
- libSceBluetoothHid: Add parameter validation to GetDeviceName, GetInputReport, SetOutputReport
- All functions follow project pattern: validation + NotImplemented_nid_no_patch
YousefMohsen0 added a commit to YousefMohsen0/AnyPS5 that referenced this pull request Oct 12, 2026
…and add parameter validation

- libSceJson2: Drop 5 functions overlapping with boykopovar#4476 (Array::iterator ctor/op++/op=, Pair ctor/dtor)
  Fix Value::clear to keep notifyError call
  Fix MemAllocator::notifyError signature (3 params)
  Fix Value callbacks signatures with self parameter
  Fix Object::insert, Array::iterator operators with proper signatures
- libSceBluetoothHid: Add parameter validation to GetDeviceName, GetInputReport, SetOutputReport
- All functions follow project pattern: validation + NotImplemented_nid_no_patch
barnacasiantiberiu added a commit to barnacasiantiberiu/AnyPS5 that referenced this pull request Oct 12, 2026
Implement iterator construction/assignment and Pair construction/destruction.
Pairs retain independent copies of nested data, and partial construction cleans
up owned state on failure. Keep postfix increment unimplemented because its
return ABI is not established; VitaSDK declares void, unlike PR boykopovar#4476.

Adapted from boykopovar/AnyPS5 PR boykopovar#4476 at
93ad413 by ahmedmakroum,
co-authored with Claude Opus 5.5. Added nested ownership, guards,
self-assignment and invalid-source regression checks locally.

Windows Release: full build and libs passed; module runner passed all six
registered module suites (nine CTests); ten integration CTests passed.
The extended guest_json test failed on unchanged sources at the missing
iterator constructor, then passed with the implementation. PC validation;
remaining ABI assumptions are recorded in TechnicalDebt.md.
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Does PPSA30601 stop at one of these five on main? With the log line this can go in; without one I'll close it until a title gets there. The PPSA30601 line in docs/dev/debt/libSceJson2.md also still lists all six Json2 members as unimplemented.

@screamm

screamm commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Windows build check (Windows 11 Pro 26220, WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release, BUILD_TESTING=ON, full build including libs): head 93ad413d merges cleanly onto main 5f426f94; guest_json 3/3; suite 683/686 plus one skip, the failures being patched_libraries (broken on main by #4877, fix #5025) and a load timeout of guest_none_relocations that passes 3/3 alone; conventions and git diff --check pass; no warnings in the changed files. No title evidence from me: Resonance is commercial. One stale line: docs/dev/debt/libSceJson2.md still says six Pair/Object/iterator members are unimplemented, while after this PR only Object::insert is left.

ahmedmakroum and others added 2 commits October 13, 2026 09:43
@ahmedmakroum
ahmedmakroum force-pushed the feat/json2-iterator-pair branch from 93ad413 to f4f34a0 Compare October 13, 2026 09:47
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#5004 docs/dev/debt/libSceJson2.md L11-12
#5438 core/libs/tests/GuestJson.cpp L612

@ahmedmakroum

Copy link
Copy Markdown
Contributor Author

Thanks for the Windows check.

No log line from me: I don’t have PPSA30601, so I can’t show it stopping at one of these five. I implemented them from the debt entry, which only says the title fails to load without them. @xXPBSXx brought PPSA30601 up in #3814; if a run of yours reaches libSceJson2, a log line would settle it.

I’ve pushed the debt fix: docs/dev/debt/libSceJson2.md now lists only Object::insert as left. If there’s no log, closing this until a title gets there is fine with me.

@oneandonlydean @screamm

@ahmedmakroum

Copy link
Copy Markdown
Contributor Author

The new overlap with #5004 is only neighbouring lines in docs/dev/debt/libSceJson2.md, no shared exports; whichever merges second can rebase it.

danielradosa added a commit to danielradosa/AnyPS5 that referenced this pull request Oct 13, 2026
Reviewer fixes: debt entry for the inferred postfix ++ return convention, value-initialised default iterator and zeroed Pair reserved bytes; drop the test assertion that only checked our own Pair layout.
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks for the work on this. Closing for now: no title reaches these five on main yet, as agreed above. You are welcome to reopen it with the title ID and the log line if PPSA30601 or another title stops at one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants