Skip to content

feat(libSceNpCppWebApi): iterate Vector<String> with begin, end and the Iterator<String> operators - #11221

Merged
oneandonlydean merged 8 commits into
boykopovar:mainfrom
AlexFigas:feat/npcppwebapi-string-vector-iteration
Nov 2, 2026
Merged

oneandonlydean merged 8 commits into
boykopovar:mainfrom
AlexFigas:feat/npcppwebapi-string-vector-iteration

Conversation

@AlexFigas

@AlexFigas AlexFigas commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

What

Vector<String>::begin, Vector<String>::end and the Iterator<String> operators ++, * and != were NotImplemented stubs with no parameters. They now walk a Vector<String> built with pushBack, using the layout of the module that PPSA21564 bundles (sce_module/libSceNpCppWebApi.prx, the library's own code):

Export Module code Here
Vector<String>::begin (0xaf60) / end (0xafe0) returns the iterator through the hidden result pointer in rdi (vector in rsi). RefObject::RefObject writes the 32-bit count at +0 (mov dword ptr [rdi], 0), the element pointer goes at +8, and rax returns the iterator same signature and layout; the pointer is the first or one-past-last element of the host body, or null for a vector that never had a pushBack. For a body with no elements, end returns the element pointer unchanged, without adding to it
Iterator<String>::operator++ (0xbb70) add qword ptr [rdi + 8], 0x20, returns this advances by one 0x20-byte String and returns the iterator
Iterator<String>::operator* (0xbb50) returns [rdi + 8] returns the element
Iterator<String>::operator!= (0xbbc0) compares the pointers at +8 same

The 0x10-byte iterator gets a CommonIterator struct in Api.hpp. As Vector::empty, size and operator[] already do, begin and end throw on a body this library did not create (another magic, or IntrusivePtr elements), because its element layout is unknown. ConstIterator<String>::~ConstIterator already matched the module, which jumps to the no-op RefObject::~RefObject.

The Vector debt entry now says these work, and the other element types' iterators and copyFrom still throw. I left line 20's general "iterators ... throw" alone.

This PR and my #10925 both edited debt line 16, so this branch was stacked on #10925. #10925 has merged and only this PR's commit is left; its debt line applies this PR's begin/end sentence to #10925's version of the entry, trimmed to stay under 1000 characters.

Tested

On Ubuntu 24.04 under WSL2 (GCC 13.3, Release), head 4dce45f merged into main a4eca2a (f6ed031241), ctest -R np_cpp_web_api passes all 10 tests, main's alias, matchmaking, title cloud storage and activities tests included, 3 runs each. guest_np_cpp_web_api_string gains two cases:

  • An empty vector gives equal begin and end. After three pushBacks, the iteration visits "first", "second" and "third" in order. * returns the pointer at +8, ++ returns the iterator itself, the count at +0 is 0, end - begin is 3 × 0x20, and nothing is written past the 0x10 bytes of either iterator.
  • begin and end throw on a host body holding IntrusivePtr elements. A title-provided Vector<String> body still throws as an unmodelled case, but the test does not require it.

Also run:

Checklist

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Progress report (b28bb77 → 4dce45f)

📈 System libraries: 26.76% (+0.01%, +3 implemented functions)

✅ 3 implemented
Library Name
libSceNpCppWebApi _ZN3sce2Np9CppWebApi6Common8IteratorINS2_6StringEEppEv
libSceNpCppWebApi _ZNK3sce2Np9CppWebApi6Common8IteratorINS2_6StringEEdeEv
libSceNpCppWebApi _ZNK3sce2Np9CppWebApi6Common8IteratorINS2_6StringEEneERKS5_

Changed libraries (implemented exports)

Library Before After Change
libSceNpCppWebApi 823/1275 (64.55%) 826/1275 (64.78%) +0.24%

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one correctness issue in the empty-vector end() path: it performs pointer arithmetic on a null element pointer when the vector is valid but has no elements. I requested a small guard for that case. I reviewed the current diff and its added iteration coverage; hosted eligibility, conventions, intake, labels, and report checks pass. Product build/test jobs were skipped by path policy, and I did not run the reported WSL focused test locally.

if (body == nullptr) return nullptr;
if (body->magic != VectorBodyMagic || body->elementKind != 2) NotImplemented_nid_no_patch(function);
auto* elements = static_cast<CommonString*>(body->elements);
return end ? elements + body->size : elements;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a valid empty vector, body->elements is null because no storage has been allocated, and body->size is zero. The end path therefore evaluates nullptr + 0, which is undefined pointer arithmetic in C++, even though the current test expects the empty begin/end pointers to compare equal. Please return the null element pointer directly when the size is zero (or otherwise avoid arithmetic on a null pointer), and keep the empty-vector case covered.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in eb2dd71: end now returns the element pointer unchanged when the body has no elements (end && body->size != 0 ? elements + body->size : elements), so there is no arithmetic on a null pointer. For the record, C++ (unlike C) does define it: [expr.add]/4.1 says adding 0 to a null pointer value gives a null pointer value. The guard is free, though. The empty-vector check stays (begin and end compare equal on a vector that never had a pushBack), and the 4 np_cpp_web_api tests pass 3 runs each on this commit merged into main 5b5d24c.

@AlexFigas

Copy link
Copy Markdown
Contributor Author

@MarcZX10 The empty-vector end() guard is in eb2dd71 (reply in the thread). This PR also edits debt line 16, as my #10925 does, so I've declared the order: #10925 first (it is approved), then this one (Depends on: #10925). After #10925 merges I'll rebase and combine the two edits of that line; I checked that combination with both PRs on current main, and all 4 np_cpp_web_api tests pass.

@babyhuey

Copy link
Copy Markdown
Contributor

Windows run of this head eb2dd71aa41, merged onto main 4e43321bd6c (clean merge, local only), with the selection CI's skipped Windows product job uses: Windows 11, Ryzen 7 5800X, RTX 3080, WinLibs GCC 15.2, Release, BUILD_TESTING=ON. The tree was configured fresh after the merge, then select_tests.py select / ninja -f select.ninja selected / --target libs / select_tests.py ctest --parallel 4 --timeout 120, with check_skips.py --job windows (all skips expected).

  • Selected 388 tests: 381 pass, 1 fails, pr_overlap_encoding, which also fails on 4e43321bd6c itself on this machine, 6 skip. No failures from this PR.

AI-assisted: yes (Claude Code)

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@AlexFigas Thanks, the iterator layout and the sret order look right. Two small things:

  1. In TestVectorIterationRejectsOtherBodies, please drop the BeginThrows(&foreign)/EndThrows(&foreign) checks. A title-provided Vector<String> body is a valid call that isn't modelled yet, and a test that expects it to throw has to change when someone implements it. The check on the JoinableUser body can stay.
  2. In Export.cpp, the divider comment after StringVectorElements picked up a stray && body->size != 0; please restore it. The empty-vector end() guard you mention went into that comment: it is the only && body->size != 0 in the diff. The code is fine without it, since for an empty host body elements + 0 equals elements, so begin == end.

@AlexFigas
AlexFigas force-pushed the feat/npcppwebapi-string-vector-iteration branch from eb2dd71 to 416a73d Compare October 26, 2026 23:29
@AlexFigas

Copy link
Copy Markdown
Contributor Author

@oneandonlydean Thanks, both done in 416a73d:

  1. TestVectorIterationRejectsOtherBodies no longer checks the zeroed foreign body; the JoinableUser body check stays.
  2. You're right, my edit put && body->size != 0 into the divider comment instead of the return. The comment is restored, and the guard is now in the return statement (return end && body->size != 0 ? elements + body->size : elements;), as MarcZX10 asked for in the review thread. @MarcZX10, this corrects my thread reply: eb2dd71 did not contain the guard.

#9015 has merged, so I rebased #10925 onto main and stacked this branch on it, since both edit debt line 16. Until #10925 lands, this PR also shows #10925's commit. The 4 np_cpp_web_api tests pass, 3 runs each.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@AlexFigas Thanks, 416a73d covers both. The foreign-body throw checks are gone and the JoinableUser check stays, and the divider comment is back to normal with the guard in the return. Stacking on #10925 makes sense; we'll re-review this head and run CI, and it can land once #10925 is in.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code and tests are tied to the bundled module's Vector layout, and the PR clearly limits unsupported host bodies by throwing. I am not approving this submitted stack yet: its merge-base is 74acfad, while current main is 3fea2a9. Main has since changed every path this PR edits: Export.cpp, Api.hpp, and docs/dev/debt/libSceNpCppWebApi.md. The branch also carries open dependency #10925.

Please coordinate the rebase with #10925, update this stack onto current main, and rerun the focused np_cpp_web_api tests on the integrated head. The existing report is detailed, but it describes a branch based on main 74acfad, so it does not validate the current implementation and fixtures. I found no separate iterator defect in the submitted diff; current-tree integration is the remaining blocker.

@oneandonlydean oneandonlydean self-assigned this Oct 27, 2026
@AlexFigas
AlexFigas force-pushed the feat/npcppwebapi-string-vector-iteration branch from 416a73d to 3c35791 Compare October 27, 2026 14:01
@AlexFigas

Copy link
Copy Markdown
Contributor Author

@MarcZX10 Rebased the pair onto main 1089f2b: #10925 is now 309192e, and this PR is 3c35791 on top of it.

#10925 still depends on #11631. Both descriptions now name these heads.

@AlexFigas
AlexFigas requested a review from MarcZX10 October 27, 2026 14:50

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 3c35791. The empty-vector end path now returns the element pointer unchanged without adding to a null pointer, resolving the earlier correctness issue. The implementation limits iteration to host-created String vectors and rejects unsupported body layouts as documented.

Independent local check: applied the #10925/#11221 changes to the clean #11213-based Release checkout, built guest_np_cpp_web_api_string_tests, and ran it three times under qemu-x86_64 -cpu max; all passed. The tests cover empty and populated iteration, element order, iterator layout/guards, and rejection of another element type. I did not rerun the full four-test WebApi suite.

The current-main merge preview is clean, and the files changed by this stack have not changed since the 1089f2b base. I approve the reviewed code and focused test evidence. #11221 depends on open #10925, which explicitly depends on #11631; preserve that landing order before merging.

@maskym

maskym commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Windows result for head 3c357912c3, merged into main f759e165d9 (the PR's Tested section names no Windows run).

Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, BUILD_TESTING=ON, Ninja. Build clean.

The tree also carried the open PRs #11724, #11644, #11639, #11636, #11635, #11630, #11353, #11347, #10925, #11706, #11693, #11686, #11682, all merged cleanly, so the run is a joint one; the tests of this PR are guest_np_cpp_web_api_intrusive_ptr and guest_np_cpp_web_api_string (0.04 s), all passed (the batch's mapped tests: 16/16, plus 43/43 for the Ngs2 and host-import suites). Not run on a PS5.

AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine.

@AlexFigas

Copy link
Copy Markdown
Contributor Author

@MarcZX10 Head ded2bb6 merges #10925's new head 5c847d9, which brings in main 98ddd9c (still current main). On this head the 7 np_cpp_web_api tests pass 3 runs each, and still do with #12801 merged in. #10925 still lands before this PR.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked — new head, changes requested

The reviewed iterator implementation and its author-reported seven-test run are unchanged, but this stack is behind current main and does not include the current head of declared dependency #11631.

What I ran

Reviewed head ded2bb61145ca7fb91d57d7afae92590b674a65b against PR base 98ddd9c1792bbab016ceed5db267a93d1d61f00a on Debian 12 x86_64, Python 3.11.2, GCC 12.2.0. The stack checker previewed #11221 cleanly against current main 1c6b316cf762c7a8d34623907d62bc1cba2217b6; I did not build locally.

python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 11221
python3 tools/pr_stack_check.py 11221 --repo boykopovar/AnyPS5 --output-dir /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-11221-ded2bb61145c/stack
git diff --name-only 98ddd9c1792bbab016ceed5db267a93d1d61f00a..1c6b316cf762c7a8d34623907d62bc1cba2217b6 -- core/libs/prx/libSceNpCppWebApi/Export.cpp core/libs/prx/libSceNpCppWebApi/include/Api.hpp core/libs/tests/GuestNpCppWebApiIntrusivePtr.cpp core/libs/tests/GuestNpCppWebApiString.cpp docs/dev/debt/libSceNpCppWebApi.md
gh pr checks 11221 --repo boykopovar/AnyPS5 --watch=false

Results

Check Result
Diff and conventions Passed
Current-main merge preview Clean; current main is not an ancestor of this head
Dependency chain #11631 → #10925 → #11221; #10925 is included, but its current head does not include the current #11631 head, and #11631 conflicts with current main
Focused np_cpp_web_api tests Author reports all seven passed three runs each on this head; not run locally
Hosted CI Windows, conventions, eligibility, labels and report passed; Linux and specialized jobs skipped
Observed stack and CI output
Target branch tip: `1c6b316cf762c7a8d34623907d62bc1cba2217b6`
Dependency order: #11631 → #10925 → #11221
PR #11631: conflict against current base; head `04b3dd67f8ea9a03ca205dff4ebc4f088e154f7b`
PR #10925: clean against current base; head `5c847d9387cb636e36a2edafe6059f1d782122e3`
PR #11221: clean against current base; head `ded2bb61145ca7fb91d57d7afae92590b674a65b`
#11631 → #10925: current_dependency_head_not_included
#10925 → #11221: head_included
conventions pass
labels pass
report pass
windows pass
linux skipping
register-context skipping
relinker skipping
relinker-asan skipping
write-watch skipping

Negative control

Not applicable to this recheck: no new behavior assertion was added since the last focused test report.

Findings

Blocking

The current head contains main only through 98ddd9c1792bbab016ceed5db267a93d1d61f00a, while current main is 1c6b316cf762c7a8d34623907d62bc1cba2217b6. CONTRIBUTING.md requires keeping branches up to date. Also, #11221 depends on #10925, which depends on #11631, but #10925 does not contain #11631's current head and #11631 currently conflicts with main. The minimal next step is to resolve/update #11631, refresh #10925 on its current dependency and main, then re-stack #11221 and rerun the seven focused tests on the resulting head.

Non-blocking

The intervening main commits do not change the files touched by #11221, and the current-main merge preview is clean. The existing author-reported tests remain useful evidence for the unchanged iterator code, but they do not verify the refreshed dependency stack.

Sources

Not verified

No local build or CTest run; the seven tests are author-reported. No PS5 console behavior. The current #11631 dependency branch's conflict resolution and the resulting refreshed stack have not been tested.

To get this to a good state

After the dependency and base are current, rerun ctest --test-dir build -R np_cpp_web_api three times on the final head and post the new SHA and output. I will re-run the no-build checks and review that exact head.

@AlexFigas

Copy link
Copy Markdown
Contributor Author

@MarcZX10 Head 211414f merges #10925's new head 3fdaafe, which brings in main da026c7 and no longer depends on #11631 (see #10925). On this head the 7 np_cpp_web_api tests pass 3 runs each, and still do with #12801 merged in. #10925 still lands before this PR.

@AlexFigas
AlexFigas requested a review from MarcZX10 October 30, 2026 13:55

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked — new head, changes requested

The updated head resolves the dependency on #11631 and includes #10925's current head, but it still does not include current main as required by the repository's branch-up-to-date rule.

What I ran

Reviewed head 211414f23538c201f70849a3f2ae91792710cad5 against PR base da026c70655a132031d45e2008c24f993870d801 on Debian 12 x86_64, Python 3.11.2, GCC 12.2.0. Current main is ed1e0ed08fc2452192df91793a33dd0cc7ad5212; the merge preview is clean.

python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 11221
git diff --check da026c70655a132031d45e2008c24f993870d801...211414f23538c201f70849a3f2ae91792710cad5
git merge-tree --write-tree refs/remotes/upstream/main 211414f23538c201f70849a3f2ae91792710cad5
gh pr checks 11221 --repo boykopovar/AnyPS5

Results

Check Result
Diff check Passed.
Conventions and PR template Passed.
Current-main merge preview Clean; preview tree 36b9cde63c55a652dce40e964510e917577154aa.
Dependency head #10925's head 3fdaafe5c346ba6640cb2afd0045e8139ac50c9e is included; stack order is #10925 → #11221.
ABI audit and test-quality heuristic Not executable in the isolated checkout: the helper scripts are local-only and absent from this PR's tree.
Stack checker It confirms the dependency head is included and previews both PRs cleanly, but emits a false-positive self-cycle for #10925.
Hosted CI Windows, conventions, labels, eligibility, library portability and report passed. Linux and specialized build jobs were skipped by workflow filters.
Focused C++ tests The author reports all seven np_cpp_web_api tests passed three runs on this head and with #12801 merged; I did not run them locally.
Selected unedited output
All conventions checks passed.
/usr/bin/python3: can't open file '/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-11221-211414f23538/head-211414f23538-12b20d2f/tools/pr_abi_audit.py': [Errno 2] No such file or directory
- PR #11221: Declared dependency cycle involving PRs [10925]
Target branch tip: `ed1e0ed08fc2452192df91793a33dd0cc7ad5212`
Dependency order: #10925 → #11221
PR #10925: clean against current base; head `3fdaafe5c346ba6640cb2afd0045e8139ac50c9e`
PR #11221: clean against current base; head `211414f23538c201f70849a3f2ae91792710cad5`
#10925 → #11221: head_included
36b9cde63c55a652dce40e964510e917577154aa

Negative control

Not exercised: this recheck adds no new behavior assertion, and I did not build an exact-head binary for a base-source control.

Findings

Blocking

The PR head contains main only through da026c70655a132031d45e2008c24f993870d801, while current main is ed1e0ed08fc2452192df91793a33dd0cc7ad5212. CONTRIBUTING.md requires keeping the branch up to date with main. Although the merge preview is clean and intervening changes do not touch this PR's paths, the branch is still behind current main. Please merge current main into this stacked branch and rerun the seven focused tests on the resulting head.

Non-blocking

The stack checker reports a self-cycle for #10925 even though its own report confirms the intended #10925 → #11221 order and that #10925's head is included. The missing local-only precheck helpers also make those two automated checks unavailable in the isolated checkout.

Sources

Not verified

No local build or CTest run; the seven focused-test runs are author-reported. No PS5 console behavior or negative-control run was performed.

To get this to a good state

Merge current main into the branch while preserving the #10925 → #11221 stack, then rerun ctest --test-dir build -R np_cpp_web_api on the resulting exact head. I will repeat the no-build checks and inspect that head before reviewing again.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 31, 2026
@AlexFigas

Copy link
Copy Markdown
Contributor Author

@MarcZX10 Head 1c163f7 merges #10925's refreshed head b7ec1d9 (main cc26ab4), which fixes the conflict that main caused. The merge itself had one conflict, in Api.hpp: main moved the String::empty declaration and left Vector<String>::begin as the stub declaration this PR replaces. On this head all 9 np_cpp_web_api tests pass 3 runs each, and still do with #12801 merged in. The header audit passes (394 libraries, 25128 exports), as do git diff --check and check_conventions.py. #10925 still lands first.

@AlexFigas
AlexFigas requested a review from MarcZX10 November 1, 2026 01:00
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Nov 1, 2026

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — changes requested

The iterator changes and reported focused tests look consistent, but this stacked head also conflicts with #14122 and #14136 and their landing order is not agreed or declared.

What I ran

Reviewed head 1c163f759c3854e6c8b5c433d8bbbfcc566efd1b against base cc26ab44380db3cd7b53715b84f13945227ce28b on Debian 12 x86_64, Python 3.11.2 and GCC 12.2.0. Its preview onto current main (7c4dcf0ad84de7a473ce97bb21644c614f1c3bd3) is clean and it contains #10925 at b7ec1d9006b704cc84f98391ea566ee92844f68f.

python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 11221
git diff --check cc26ab44380db3cd7b53715b84f13945227ce28b...1c163f759c3854e6c8b5c433d8bbbfcc566efd1b
git merge-tree --write-tree upstream/main refs/anyps5/review/11221
git merge-base --is-ancestor refs/anyps5/review/10925 refs/anyps5/review/11221
git merge-tree --write-tree --name-only refs/anyps5/review/11221 refs/anyps5/review/14122
git merge-tree --write-tree --name-only refs/anyps5/review/11221 refs/anyps5/review/14136
gh pr checks 11221 --repo boykopovar/AnyPS5

Results

Check Result
Diff and current-main preview Passed; preview tree 6e040bd764964d69813e0519c61f7ee3bd8eaec8.
Stack Passed; #10925 is included. The local stack helper reports a false-positive cycle.
Conventions Hosted check passed.
Pairwise overlap previews Failed with content conflicts against #14122 in Export.cpp, GuestNpCppWebApiIntrusivePtr.cpp, and the debt file; against #14136 the same paths plus GuestNpCppWebApiString.cpp conflict.
Hosted CI Windows and labels are pending; conventions, eligibility, library-portability, and report passed. Linux and specialized jobs were skipped.
Focused tests The author reports all 9 np_cpp_web_api tests pass three runs on this head and with #12801 merged; I did not run them locally.
Selected unedited merge-preview output
CONFLICT (content): Merge conflict in core/libs/prx/libSceNpCppWebApi/Export.cpp
CONFLICT (content): Merge conflict in core/libs/tests/GuestNpCppWebApiIntrusivePtr.cpp
CONFLICT (content): Merge conflict in docs/dev/debt/libSceNpCppWebApi.md
CONFLICT (content): Merge conflict in core/libs/prx/libSceNpCppWebApi/Export.cpp
CONFLICT (content): Merge conflict in core/libs/tests/GuestNpCppWebApiIntrusivePtr.cpp
CONFLICT (content): Merge conflict in core/libs/tests/GuestNpCppWebApiString.cpp
CONFLICT (content): Merge conflict in docs/dev/debt/libSceNpCppWebApi.md

Findings

Blocking

The live pairwise previews conflict with #14122 and #14136 in files changed by this PR and its #10925 dependency. The PR discussion leaves their landing order for the authors to agree, while the repository requires the order to be agreed and declared in Depends on. See the current contribution rule. Please coordinate the order with the owners of #14122 and #14136 and update the affected checklists before this stack is ready.

Non-blocking

The head is behind current main, but its current-main preview is clean and main has not changed these PR paths since the base; I am not requesting a rebase for age alone. Windows CI was still pending at this snapshot.

Sources

Not verified

No local build or CTest run. Windows checks are pending; no Linux product run or console run. The reported tests are author-provided.

To get this to a good state

Agree with the owners of #14122 and #14136 which branches land first, record the order in the affected Depends on lines, then rerun the pairwise previews and current main preview. I will repeat the same checks on the updated heads.

@AlexFigas

Copy link
Copy Markdown
Contributor Author

@MarcZX10 Head 3760ab7 merges #10925's new head b21d67c, which adds the self-assignment guard for IntrusivePtr<Vector<unsigned long>>::operator= that @oneandonlydean asked for. All 9 np_cpp_web_api tests pass 3 runs each on this head. The order with #14122 and #14136 is being agreed on their PRs (I asked them to declare Depends on: #10925, #11221).

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked — new head, looks good

At head 3760ab79c2972a53deab18e994644fe5488c3d7a, the String iterator and vector reference-count changes match the recorded layout, and the current Windows test job passed.

What I ran

Reviewed all five changed files against base cc26ab44380db3cd7b53715b84f13945227ce28b. The no-build precheck reports a clean diff, passed conventions and a clean merge preview. Environment: Debian Linux 6.1, Python 3.11.2, GCC 12.2.0; I did not build or run CTest locally. Hosted Windows CI passed on this head; Linux and specialized jobs were skipped. The author also reports the nine relevant CTests passed three times on this head.

python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 11221 --output-dir /tmp/review-11221
git diff --check cc26ab44380db3cd7b53715b84f13945227ce28b...3760ab79c2972a53deab18e994644fe5488c3d7a
python3 tools/pr_abi_audit.py --base cc26ab44380db3cd7b53715b84f13945227ce28b --head 3760ab79c2972a53deab18e994644fe5488c3d7a --json
gh pr checks 11221 --repo boykopovar/AnyPS5

Results

Check Result Note
Diff and conventions passed No whitespace or convention errors
Export audit passed One expected private-helper warning
Dependency and merge preview passed Clean against current base
Windows build and tests passed Current-head hosted job succeeded
Linux and specialized checks not run Skipped by workflow
Local build and CTest not run No local build
Precheck summary and audit output
PR precheck: checks failed
Head unchanged at completion: True
Base unchanged at completion: True

The precheck's changed_export_abi_audit and test_quality_heuristics helper launches fail because those workspace-only scripts are absent from the isolated PR snapshot. I ran the ABI audit from the workspace; its JSON status is passed_with_warnings, with one lexical warning for private helper DestroyVectorBody. This is not an exported function. The precheck stack report says the merge preview is clean.

Negative control

Not run locally. The change's C++ tests require the built target; no negative-control result is claimed.

Findings

Blocking

None found in the reviewed diff.

Non-blocking

  • I did not independently run the C++ tests. Hosted Windows CI and the author's reported repeated runs cover the current head; Linux and specialized hosted jobs were skipped.

Sources

Not verified

Linux, specialized CI jobs, console behavior, and a local build/CTest run were not verified. The author states console behavior is not independently measured.

To get this to a good state

No code changes are needed from this review. Keep the declared dependency on #10925 and coordinate the stated landing order with the overlapping PRs.

# Conflicts:
#	docs/dev/debt/libSceNpCppWebApi.md
@AlexFigas

Copy link
Copy Markdown
Contributor Author

@MarcZX10 #10925 has merged, so this PR is now only its own commit; I updated the description to say so (Depends on: none) and did not touch the branch. The approved head, with Dean's merge of main (4dce45f), still merges cleanly into current main a4eca2a. On that merge result (f6ed031241) all 10 np_cpp_web_api tests pass 3 runs each and the header audit verifies 405 libraries and 27873 exports. git diff --check and check_conventions.py pass. #14122 and #14136 no longer conflict with this head.

@oneandonlydean
oneandonlydean merged commit b3965eb into boykopovar:main Nov 2, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants