Count the compiler warnings in a ParparVM native build, by owner - #5750
Open
shai-almog wants to merge 24 commits into
Open
Count the compiler warnings in a ParparVM native build, by owner#5750shai-almog wants to merge 24 commits into
shai-almog wants to merge 24 commits into
Conversation
A ParparVM build compiles four different kinds of C into one binary -- the translator's output, the ParparVM runtime, the hand-written port natives, and vendored third-party sources -- and their warnings arrive in one log with nothing to tell them apart. At that volume a real defect is invisible: an Apple API that is deprecated now and deleted in two releases, or a pointer/integer confusion in generated code, reads the same as the noise around it. Nothing in this tree has ever counted them, and the Android port has already shown what that costs (API 37 deleted FingerprintManager with every check green). Ownership cannot be recovered from the path. ByteCodeTranslator.execute() copies every non-class file into the same flat srcRoot as the generated code, so CN1Vision.m and com_codename1_ui_Form.m are indistinguishable siblings, and bytecode-translator-files.txt is a plain find over that directory. So the translator records provenance at each copy site into cn1-source-manifest.txt. It goes in the project root rather than srcRoot deliberately: getFileType() has no case for .txt, so anything left in srcRoot falls through to ***RESOURCES*** and is copied inside the shipped .app. check-native-warnings.py reads that manifest plus a build log and attributes every diagnostic to generated / runtime / port / vendored / sdk / toolchain, holding the result against a per-leg baseline in the manner of check-cast-semantics.sh. No baseline is committed yet -- one has to be frozen from a real CI leg, not from a local run. Three guards, because a gate that reads nothing reports success: - The build must have compiled everything the manifest lists. An incremental build recompiles nothing, reports no warnings, and is indistinguishable from a clean codebase; so is the documented ARCHS failure where xcodebuild "silently compiles NOTHING while still copying resources". - --probe injects one synthetic warning and asserts the whole chain reacts, on the real log with the real manifest, so a gate that has gone blind fails the day it breaks rather than the day someone notices it never fired. - --self-test checks the parser against a hand-authored fixture and the baselines against their own format. Both run in pr.yml and need no compiler. The census runs on scripts-ios.yml's build-ios leg because that is the one that can: it sets no CN1_IOS_DERIVED_DATA, so it wipes derived data and compiles cold, and its path filters are a superset of the other iOS legs'. It is gated on CN1_WARNING_CENSUS, which our workflows set and nothing a customer runs does, and it is report-only for now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0be6104ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Xcode names the source it is about to compile; ninja and make announce the | ||
| # object. Either way this is how we learn what the build ACTUALLY compiled, as | ||
| # opposed to what it could have compiled. | ||
| COMPILE_XCODE_RE = re.compile(r'^\s*CompileC\s+(?:"[^"]*"|\S+)\s+(?P<src>"[^"]+"|\S+)\s+normal\b') |
There was a problem hiding this comment.
On the checked build-ios workflow, CN1MetalShaders.metal is copied from Ports/iOSPort/nativeSources and included in the Xcode sources, but Xcode announces it with a CompileMetalFile operation rather than CompileC. Since this regex is the only Xcode compilation grammar while .metal is included in SOURCE_EXTS, check_completeness() always reports that shader as uncompiled and exits 2; run-ios-ui-tests.sh then swallows that status, so this leg produces no warning census or JSON artifact despite a successful cold build.
Useful? React with 👍 / 👎.
Comment on lines
+1164
to
+1167
| // Only the one-file-per-class case has a file name worth recording. Under | ||
| // concatenation the classes are bucketed into concatenated_<n> and the | ||
| // provenance of an individual class is genuinely gone by the time the | ||
| // compiler sees it; the manifest says so by simply not naming these. |
There was a problem hiding this comment.
When concatenateFiles=true—used by iOS superfast builds and unconditionally by MacOSNativeBuilder—the compiler sees concatenated_<n>.m, but this branch deliberately records none of those generated files. Any warning emitted from a concatenated unit consequently has no manifest entry or external-path marker, so classify() treats it as unattributed and aborts the census instead of placing it in the generated group; record each nonempty concatenated output when realClose() creates it.
Useful? React with 👍 / 👎.
The completeness guard asked the wrong question. It required every source in the manifest to appear as compiled, but the manifest names every file in the generated project and a target legitimately builds a subset: a .metal goes through CompileMetalFile rather than CompileC, and a source can be excluded from a target outright. Demanding all of them would have failed the first real census for a reason that is not a defect. The thing actually worth catching is different: a build that compiled LESS than the one the baseline was frozen from. That is what an incremental build looks like, it reports no warnings, and it is indistinguishable from a clean codebase. So the ratchet is on coverage, recorded in coverage-<leg>.txt beside the baseline. It is exact, needs no threshold, and needs nobody to enumerate which files a given target happens to include. A build that compiles nothing at all is still fatal on its own. Sources the manifest lists that this target never builds are now reported rather than fatal, which is the honest reading of them. Also recognises the real task-line shape, checked against Xcode 26.3 output rather than assumed: the task name is followed by output, source, "normal", the arch, and a trailing "(in target ... from project ...)". Both that line and the CompileMetalFile form are in the fixture now, so neither can regress silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
METALView.h and METALView.m exist in both Ports/MacPort and Ports/iOSPort/nativeSources, so resolving a warning's file by name alone was ambiguous and would have refused the macOS leg outright the first time either one warned. The leg already answers it: a macOS build compiled the MacPort copy. Listing each leg's port trees most-specific-first states that rather than guessing at it, and a name claimed by a more specific tree is not reconsidered. A name that appears twice inside a SINGLE port tree still refuses to resolve. There the leg tells us nothing, and picking one would put a file nobody edited into the baseline. Also indexes each port tree once instead of walking it per diagnostic, which was O(diagnostics x files) against a census that carries thousands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"/Library/Developer/" also matches a developer's own ~/Library/Developer/Xcode/DerivedData, so a Swift package checkout under SourcePackages was being labelled an Apple SDK header. The vendored markers are the more specific ones and belong first. Neither group gates, so no verdict changes -- but the census exists to be read, and one that misattributes what it reports is not worth reading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The summary table had a row per distinct warning kind, and the iOS leg carries far more of them than the clean target does. GitHub drops a step summary whole once it exceeds 1MB, so an unbounded table risks costing the entire report rather than its tail -- the failure mode being a census that ran, found everything, and showed nothing. Capped per group, with the hidden rows counted rather than silently dropped, and a line saying the tail is in the JSON dump. The census itself is unchanged; this only bounds what a human is asked to scroll. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Cloudflare Preview
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 584caa96d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * somebody else's vendored code. Written out beside the generated project at the | ||
| * end of each output handler; see {@link SourceManifest}. | ||
| */ | ||
| static final SourceManifest sourceManifest = new SourceManifest(); |
There was a problem hiding this comment.
When ByteCodeTranslator.main() is invoked more than once in the same JVM—as the integration tests and embedded callers can do—this static manifest retains every entry from the previous application because neither main() nor Parser.cleanup() clears it. The next project therefore receives a manifest containing sources that do not exist in its output, and retained origins can also classify a same-named file using provenance from the earlier build; create or clear the manifest at the start of each translation.
Useful? React with 👍 / 👎.
| // globs srcRoot for sources, and the Apple path lists it into the Xcode project, | ||
| // where an unrecognised extension lands in the resources phase and ships inside | ||
| // the bundle. See SourceManifest. | ||
| sourceManifest.write(root); |
There was a problem hiding this comment.
For the windows and linux clean targets, embedWindowsResources()/embedLinuxResources() always generate cn1_resources_table.c and may also generate cn1_resources_data.S, but neither file is added to sourceManifest before it is written here. A compiler warning in either generated unit consequently has no manifest entry, so classify() treats it as unattributed and aborts the warning census instead of assigning it to generated code.
Useful? React with 👍 / 👎.
Collaborator
Author
|
Compared 166 screenshots: 166 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 166 screenshots: 166 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 166 screenshots: 166 matched. Benchmark ResultsDetailed Performance Metrics
|
SpotBugs DM_CONVERT_CASE, and it is the tree's documented rule rather than a style nit: String.toLowerCase() is locale sensitive, and Codename One has no java.util.Locale to ask for the root one, so the fold a device performs depends on who is holding it. A protocol token another program parses back is exactly the case that must never be produced by folding. The four current constants happen to contain no dotted I, so nothing was broken today -- but the next one added could, and it would fail only for users whose device is set to Turkish or Azerbaijani, with nothing throwing. Writing the token out also means the manifest's wire format is stated in the enum rather than being an accident of the Java identifier, so renaming a constant can no longer silently change the file that check-native-warnings.py parses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74fe911480
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+44
to
+46
| GNU_RE = re.compile( | ||
| r'^(?P<path>[^\s][^:]*(?::[^:\s][^:]*)*?):(?P<line>\d+):(?P<col>\d+):\s+' | ||
| r'(?P<sev>warning|error|note):\s+(?P<msg>.*)$') |
There was a problem hiding this comment.
When a compiler emits a diagnostic in the valid path:line: warning: ... form without a column, this pattern does not match because it requires :line:col:, while BARE_RE only accepts warnings beginning at column zero. Such warnings are silently omitted from the JSON and baseline comparison, allowing the census to report clean despite warnings in the build log; make the column portion optional and cover this form in the parser fixture.
Useful? React with 👍 / 👎.
Comment on lines
+687
to
+691
| ByteCodeTranslator.sourceManifest.recordGenerated("cn1_class_method_index.h"); | ||
| fos = new FileOutputStream(new File(outputDirectory, "cn1_class_method_index.m")); | ||
| fos.write(bldM.toString().getBytes(StandardCharsets.UTF_8)); | ||
| fos.close(); | ||
| ByteCodeTranslator.sourceManifest.recordGenerated("cn1_class_method_index.m"); |
There was a problem hiding this comment.
When cn1.onDeviceDebug=true, writeSymbolSidecar() emits and the generated project compiles cn1_debug_symbols.c, but unlike the generated files recorded here, that sidecar is never passed to sourceManifest.recordGenerated(). A warning in it therefore has neither a manifest entry nor an external-path marker, so classify() marks it unattributed and aborts the census instead of assigning it to generated code; record the sidecar when it is emitted.
Useful? React with 👍 / 👎.
Collaborator
Author
|
Compared 166 screenshots: 166 matched. |
Collaborator
Author
|
Compared 166 screenshots: 166 matched. |
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 160 screenshots: 160 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
The first real census of an iOS build: 92,129 warning lines in a 1.17M-line log, none of which anything had ever counted. 60,952 of them are ours and gating, across 157 distinct kinds. Coverage confirms it came from a cold build -- 3,147 sources compiled, none of the manifest's 3,133 skipped. What the split by owner buys is visible immediately. Generated code is four emitters, two of which account for essentially all of it: 50,967 -Wunused-variable and 9,094 -Wincompatible-pointer-types-discards-qualifiers. The 814 warnings in the hand-written port were the ones worth finding, and they were unreadable underneath that: 470 deprecations (OpenGLES, and MPMoviePlayerController asking for AVPlayerViewController), 67 -Wshorten-64-to-32, 21 -Wint-conversion, 10 ARC bridge casts in non-ARC code, 6 -Wunsupported-availability-guard -- availability checks that do not guard -- and 4 -Wundeclared-selector, which is the class the macOS template already makes an error because it crashes on the device. Three log-transport defects had to be fixed first, all found in the real log rather than imagined. xcodebuild's output reaches the log through a pipe and a long diagnostic can arrive broken at an arbitrary byte: - split in the PATH, leaving a file name like "odename1_ui_Display.m" that belongs to nothing; - split in the MESSAGE, which is worse because the first half still parses and yields a message shape of "unuse" -- a baseline row that could never match again; - and occasionally bytes are LOST rather than split, so the path is gone outright. Those are counted and reported as lost, never attributed and never baselined: a row keyed on no file cannot recur, so baselining one would guarantee a stale entry later. Both joins are self-validating rather than guessed -- a path join must produce something that parses, a message join must complete a trailing flag that was absent. All three shapes are now in the parser fixture. A fileless diagnostic carrying a [-Wflag] is one of the lost ones, not a build-system warning; clang flags belong to file-scoped diagnostics, and that is what separates it from a genuine "Skipping duplicate build file". Two provenance rules the census asked for: an embedded watch or tv app is a second, independent ParparVM translation writing to a sibling -src directory, so its output is generated code by the same argument as the main app's; and an .xcframework is unzipped into the build's own products directory before its headers are compiled, so those arrive under a local-looking path and are not ours. The leg now gates rather than reporting, and runs --probe afterwards so a gate that has gone blind fails the same day. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40e9fd9c40
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
The gate's first live run failed on generated|*|<no-flag>|unused variable ?, which is the flake this was warned about rather than a new warning: a diagnostic split immediately before a space, leaving the continuation as " [-Wunused-variable]". Excluding every indented line as "probably a source snippet" excluded that too, so the diagnostic kept a truncated message and lost its flag. Only clang's actual snippet and caret shapes are excluded now; the join still has to complete a trailing flag, so nothing else can be glued on. Two runs of the same leg then produced identical baselines -- 157 keys, same set, from logs of 60,952 and 61,124 gating diagnostics. That is the property the ratchet needs and it is now measured rather than assumed. Getting there exposed two holes, both of the "gate reads nothing and reports success" kind this whole exercise exists to prevent: An unknown leg name resolved to no port trees, so every hand-written native failed to resolve, fell through to vendored, and left the gating set. The census still printed and still passed, having quietly stopped checking the code most worth checking -- 144 of 157 entries. It is a hard error now; a leg with no port sources says so with an empty list, as clean-target does. And the coverage ratchet compared against the sources the baselined build compiled, which couples the gate to something that legitimately moves: the two runs compiled 3,147 and 3,156 sources. Each compiled 100% of its OWN manifest, which is the invariant that actually holds, needs no stored state, and catches an incremental build just as well. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eae4f3443f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…hing reads 51,108 of the iOS leg's 61,124 gating warnings were -Wunused-variable in generated code, and none of them came from where the plan guessed. The census data named the actual sources, which is the whole reason it was built before any of this was attempted: SP 21,577 currentCodenameOneCallStackOffset 8,727 methodBlockOffset 8,727 currentOffset 6,495 locals 3,046 objInstance 2,438 The first five are declared unconditionally by the six frame macros, because a macro cannot know whether the method body it precedes will touch the operand stack, enter a try block, or have locals at all. Emitting a different macro per combination would multiply the frame variants without making any generated code better, so the declarations say so instead: CN1_UNUSED is the C idiom for exactly this, and it suppresses only these variables, leaving a genuinely unused variable anywhere else reported. It is keyed on the compiler feature rather than the vendor. clang-cl defines _MSC_VER as well as __clang__, so testing for MSVC first -- the shape the neighbouring CN1_NORETURN uses -- would have silently dropped the attribute on the Windows port and left that leg as noisy as before. objInstance is different and is a real emitter fix: __GC_MARK_ casts objToMark for every class, but a class declaring no object fields of its own marks nothing and only chains to its base, so the cast is a variable no statement in the function reads. Thousands of classes hold nothing but primitives. Every use of it sits inside the field loop -- both the base-class call and the java.lang.Object branch use objToMark -- so the guard is the loop's own condition. Measured on a clean-target build: 3,802 warnings to 2,800, with generated -Wunused-variable going from 1,002 to zero. Verified in the generated C both ways: java_lang_String still casts and marks its field, java_lang_Integer emits neither. The baseline is unchanged on purpose. Roughly 98 -Wunused-variable instances survive in generated code from other emitters, so the row still reproduces -- which is what keying the ratchet on presence rather than count is for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… for 9,125 warnings on the iOS leg, all one message: passing 'struct elementStruct *volatile *' to a parameter of type 'struct elementStruct **' discards qualifiers. The compiler was right, and this is the defect the census was built to surface rather than noise to be quietened. POP_INT() and friends expanded to (*pop(&SP)).data.i, and pop() takes struct elementStruct**. A method that emits setjmp declares SP volatile so longjmp cannot clobber it -- that is the entire reason the _VSP frame variants exist -- and &SP is then struct elementStruct *volatile *. So on exactly the frames where the qualifier matters, every pop handed the runtime a pointer that discarded it, and a write through that pointer is not the volatile access the declaration asked for. (*--SP) is the same decrement-then-dereference without taking SP's address, so the qualifier stays where the frame put it. popMany had to move SP by an amount it computes from the slot types, so it could not just drop the pointer: it becomes cn1PopMany, value in and value out, and POP_MANY assigns the result back. Where SP is volatile that assignment is an ordinary volatile store, which is what was wanted all along. Nothing else called either one. The two remaining pop/popMany spellings in cn1_globals.h sit inside a commented-out block and are dead. Verified: the full ParparVM suite, 557 tests, no failures, with CleanTargetIntegrationTest running its whole 35 -- these compile and RUN translated programs, which is the check that matters for a change to the hottest path in the VM. On a clean-target build the category went from 75 to zero and total warnings 2,800 -> 2,725; the generated C carries no pop(&SP) anywhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d4e39fd1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Five CLANG_WARN_* settings are off in the iOS Xcode project, presumably to quieten the translated C. The census runs with all five forced on, so what each one actually costs is now measured rather than assumed, across a full application build -- generated code, the ParparVM runtime, the iOS port natives and the pods: CLANG_WARN_EMPTY_BODY 0 CLANG_WARN__DUPLICATE_METHOD_MATCH 0 CLANG_WARN_ENUM_CONVERSION 10 (all in the port) CLANG_WARN_INT_CONVERSION 30 (28 port, 2 runtime) GCC_WARN_UNUSED_VARIABLE ~180 after the emitter fixes The first two find nothing anywhere. They have been suppressing no diagnostic at all, so turning them on costs this build nothing and gives an application author the same two checks over their own native code. Both are already YES on the app-extension targets the builder writes, so the main target was the odd one out. The other three do still fire, and stay off until what they find is fixed rather than being turned on to add warnings to every customer build. -Wint-conversion is the one worth reading first: in current clang it defaults to an error, so = NO has been actively suppressing a pointer/integer confusion class in code that ships. Left alone in the macOS project, which carries the same five. Its census has not been taken, the port natives there are different files, and zero on iOS is not evidence about MacPort. Verified on a regenerated project: both land as YES, every literal IPhoneBuilder anchors its regex rewrites on is intact, and injectDevelopmentTeam still matches both SDKROOT blocks whole -- the added comment carries no nested "};" that would truncate one, and no "template" that replaceInFile would rewrite into the app name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca943a9733
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The Linux screenshot suite does not deadlock. The event dispatch thread dies and
the process keeps running:
[EDT] Exception: java.lang.ArrayIndexOutOfBoundsException - -1567458274
at com_codename1_ui_Display.callSeriallyOnIdle:1174
...
java.lang.NullPointerException
at com_codename1_ui_util_EventDispatcher.fireActionEvent:305
at com_codename1_ui_Display.mainEDTLoop:1611 <- inside the catch block
at com_codename1_ui_RunnableWrapper.run:139
at com_codename1_impl_CodenameOneThread.run:179
at java_lang_Thread.runImpl:216 <- top frame of the EDT
An exception reaches mainEDTLoop's catch. The catch reports it through the
application's error handler. The handler throws. That second throwable
propagates out of the catch, out of the dispatch loop and off the end of the
thread -- the trace reaches Thread.runImpl, so the thread is gone. Every other
thread lives, so the process stays up; nothing paints or handles input again.
The suite then sits idle until a 40-minute cap kills it with 13 of 100
screenshots never taken.
Four of the calls in that block run code Display does not own: a registered
CrashReport, the port's handleEDTException, the application's error handler, and
Dialog.show -- which paints, so it fails for any reason painting fails. Any one
of them ending the dispatch thread is the same permanent freeze, and this is
reachable from ordinary application code rather than being specific to CI.
There were TWO such blocks, not one: the dispatch loop's, and the phase that
runs before the first Form is shown. Both now go through reportEdtException,
which cannot let a second throwable escape. Both throwables are logged, the
original first so it is not buried by the failure to report it.
Unifying them means the pre-Form phase now also calls
CodenameOneThread.handleException, which it did not before. That is deliberate:
the method only acts when Log.isCrashBound(), so a crash-bound application was
silently losing exceptions from that phase.
EdtExceptionReportingTest covers one collaborator per test rather than one
representative case, because the hazard is per call site and wrapping three of
four would look fixed. Verified non-vacuous: with the guard removed all four
fail, each naming its own collaborator -- including the default path, where an
application that registers nothing still dies, there on
ExceptionInInitializerError out of Dialog.show.
What this does NOT fix is whatever produced the original garbage exception. That
is pre-existing (identical stack on PR #5741), x86_64+glibc only (arm64 and musl
pass in the same run from the same zig cc), and the exception TYPE varies
between occurrences at a fixed site, which is the signature of reading garbage
rather than a logic error; the stack is semantically impossible too, since
callSeriallyOnIdle is not called from paintDirty. With this change the EDT
survives it, so the next occurrence logs and the suite continues instead of
stalling -- which is what will make that corruption diagnosable.
Core suite: 6,617 tests, no failures.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7923a81853
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Four occurrences of the Linux suite stall produced a hang-stacks.txt containing seven sample headers and zero stacks. That is why the stall still has no root cause: gdb is absent from the runner image, the install is best-effort with its output discarded, and the IOException from ProcessBuilder.start() was caught by a bare "catch (Exception ignore)". The file existed, was uploaded, and read as though nothing had gone wrong. A diagnostic that can quietly produce nothing is not a diagnostic. Both halves now report: - The capture compares the file length before and after gdb and, when nothing was written, says so together with gdb's exit code and whether gdb runs at all. Those two failure modes need different fixes -- a missing binary is a runner-image problem, a present binary that captured nothing is a ptrace or attach problem -- and they are indistinguishable in an empty file. - The workflow no longer discards the install output, and states whether gdb ended up on PATH. Still best-effort in both places, deliberately: a runner without gdb must not turn a hang into a second, more confusing failure. It must just stop pretending it looked. gdbOnPath() verified against ground truth rather than assumed -- it answers false on a machine with no gdb. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b4b816e7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The 0x100000028 crash was diagnosed as far as it was only because this handler prints faultAddr itself. Nothing downstream could have recovered it: the suite is built -O3 with LTO, so gdb's post-mortem answers "No locals" and "No arguments" for every frame of the core and reports every global as "unknown type". The one piece of evidence that named the defect class came from here, and everything the next step needs is missing from here too. Two additions, both async-signal-safe and neither reading the faulting address, which is unmapped by definition: - The integer registers. They cost nothing and touch no memory, and they are the only place the faulting POINTER survives -- the address says a read went wrong, the register file says what the value WAS and which other register held the object it should have come from. - The top 32 words of the faulting frame, clamped to the real stack top so it reads only mapped memory and cannot fault a second time. The stack words are what separate the two remaining explanations for this crash. An address whose low 32 bits are zero and bit 32 is set (0x100000000 here, plus a 0x28 field offset) is either a 64-bit value read as an object pointer or a 32-bit write into a 64-bit object slot, and those are indistinguishable from the address alone. An adjacent word holding the other half of the same 64-bit value says the first; an intact pointer beside a clobbered one says the second. The buffer sizing and read bounds are tested rather than eyeballed -- a signal handler that overruns a stack buffer is worse than the fault it is reporting. The worst case uses 23 bytes of the 64-byte buffer, and the frame dump clamps to 5 reads when only 5 words remain below the stack top. Compiles only on the Linux legs, so CI is the first compile of the arch-specific halves; the portable logic was exercised locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gdb's post-mortem of the SIGSEGV answered "No locals" and "No arguments" for
every frame and "unknown type" for every global. That was read as LTO stripping
the information. It is not: the generated project compiles with -g1, which emits
line tables and function names and NOTHING about variables or types.
Measured rather than assumed, at -O3 on a struct-and-locals probe:
-g1 : variable/parameter DIEs = 0 , type DIEs = 0
-g3 : variable/parameter DIEs = 4 , type DIEs = 3
So the core was always fine and the DWARF simply did not describe the data. No
amount of post-mortem gdb could have recovered it, which is why four occurrences
produced a backtrace and never an explanation.
-g1 remains the default -- it is right for a shipping binary, where the companion
exists to turn an address back into a Java method, and the level is now a cache
variable so an unset build is byte-identical to before. The Linux suite sets
CN1_LINUX_FULL_DEBUG and gets -g3 into the <exe>.debug companion; the binary
itself is still --strip-all'd, so only the companion grows.
The core is also kept now. crash-stacks.txt is only ever the backtrace gdb could
produce in place; printing the object a faulting pointer came from, the slot
beside it, or the register that held it needs the core in hand. It is uploaded
with the stripped ELF it refers to and the .debug companion that decodes it,
compressed (zstd where available), and collected only when a core exists -- which
is only when the suite actually crashed. A README beside it gives the two
commands to open it.
This is what the outstanding question needs: 0x100000028 is either a 64-bit value
read as an object pointer or a 32-bit write into a 64-bit slot, and telling those
apart means reading the neighbouring slot, which means decoding the core.
Verified end to end on a scratch project: default configure compiles -g1, and
-DCN1_DEBUG_INFO_LEVEL=3 compiles -g3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gate failed, and it failed correctly: one baselined warning kind no longer reproduces, so the ratchet demanded the row be deleted rather than left to describe a build nobody produces any more. generated|*|-Wincompatible-pointer-types-discards-qualifiers|passing ? to parameter of type ? discards qualifiers That is the 9,125-instance category the operand-stack POP change removed. It is gone from the iOS build entirely. The S4 emitter work, measured on CI's own build rather than the local clean target: warnings in the log 92,129 -> 2,201 gating diagnostics 61,124 -> 989 generated (distinct) 12/60,107 -> 3/144 -Wunused-variable in generated code is 98, which is what was predicted when the baseline was deliberately left alone: the row survives because a residual from other emitters still reproduces, and keying the ratchet on presence rather than count is what kept that from being churn. The port's 814 warnings are now the largest gating group. That was the point -- 470 deprecations, 67 -Wshorten-64-to-32, 21 -Wint-conversion and 6 -Wunsupported-availability-guard were always there and were unreadable under 60,000 lines of generated-code noise. Verified against this run's own log and manifest before pushing: the gate passes with 156 entries and the injected-warning probe still fires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1e5bf9cc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
18 of the iOS port's 21 -Wint-conversion findings are one line repeated: a GLuint initialised with NULL. A GL object name is an unsigned integer handle, not a pointer, and 0 is its documented "no object" value; NULL is a pointer constant, and initialising an integer with it is exactly what the warning reports. Nothing was broken -- the two are the same bits on every target this builds for -- but the compiler is correctly refusing to assume that, and the setting that reports it (CLANG_WARN_INT_CONVERSION) is one of the five the iOS project turns OFF. In current clang -Wint-conversion defaults to an ERROR, so what that setting has been suppressing is a class that includes genuine pointer/integer confusion; these 18 were the noise standing between it and being turned back on. Left for their own change: the handful of JAVA_LONG <-> void* sites in IOSNative.m and CodenameOne_GLViewController.m. Those are the deliberate "keep a native peer in a Java long" idiom and want an explicit cast each, decided site by site rather than swept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acb6732339
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Second time the stale-entry rule has fired, and again correctly: port|Ports/iOSPort/nativeSources/CN1ES2compat.m|-Wint-conversion|incompatible pointer to integer conversion initializing ? (aka ?) with an expression of type ? Initialising those 18 GL object names with 0 rather than NULL removed the kind from that file entirely, so the row described a warning the build no longer emits. -Wint-conversion across our code is 30 -> 12. Verified against the run's own log and manifest: the gate passes with 155 entries and the injected-warning probe still fires. Worth noting what this rule is buying. A baseline that only ever grows is an allow-list; requiring that a fixed warning be deleted is what keeps it a record of real remaining debt. Both times it has fired, the correct action was one line removed and nothing else -- which is the cost it is meant to have. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The last nine -Wint-conversion sites in our code, each read rather than swept --
they are not one pattern, and treating them as one would have papered over the
two that were plain mistakes.
Four are a pointer constant used where an integer zero was meant, the same class
as the GL object names:
defaultDatePickerDate = nil x3 (a JAVA_LONG; the same file already
writes `= 0` for it two lines away)
__block JAVA_LONG outTexture = NULL
Four are the deliberate "keep a native peer in a JAVA_LONG" idiom, which is
sound on every 64-bit target this builds for and was simply never stated. The
read side already casts back -- `(void*)s->java_lang_String_nsString`,
`(Renderer*)(uintptr_t)renderer` -- so only the write side was silent:
editStringAtImpl(..., n5, ...) n5 is a long, the parameter is void*
return (BRIDGE_CAST void*)resultGl from a JAVA_LONG function
networkError(..., peer, str) peer is a void*, the parameter is long
nnn->java_lang_String_nsString = str an NSString* into a JAVA_LONG field
One was both a missing cast and a dead local: the block binds
`Renderer *r = (Renderer*)(uintptr_t)renderer` and never uses it, then passes
the uncast `renderer` to Renderer_produceAlphas. Using `r` fixes the conversion
and the unused variable together.
Deliberately untouched: `currentDatePickerDate = nil` in the same function, eight
times. That one really is an NSDate*.
This is what CLANG_WARN_INT_CONVERSION = NO has been hiding. In current clang
-Wint-conversion defaults to an ERROR, so the setting suppresses a class that
includes genuine pointer/integer confusion; nothing here turned out to be a live
bug, but that could only be established by reading all of them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e96c4253fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
build-test (8) failed with exit 124: `timeout 300 apt-get update` spent its whole budget and was killed. The step runs under `-e`, so the job ended before Maven, every module's SpotBugs report was therefore missing, and the quality gate failed -- correctly, since a missing report means the analysis never ran -- for want of an analysis rather than a finding. scripts/ci/apt-get-install.sh exists for exactly this and three steps were not using it. It bounds AND retries the index refresh, and it skips the apt round trip entirely when every requested package is already present. libdbus-1-3 is baked into the pr-ci-container image, so the common case becomes no network at all -- this failure could not have happened through the helper. All three converted rather than the one that failed today: sqlcipher and gcc/gcc-mingw-w64-x86-64 carry the identical pattern and the identical exposure, and their comments already said "bounded for the same reason as the libdbus install above". Bounding was the previous answer to this and it is not enough on its own -- a bounded hang is still a dead job, just a punctual one. The helper uses sudo, which the container image installs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A code review raised eighteen findings on this PR. These are the ones that were
real, each verified against a real CI log rather than reasoned about:
PROVENANCE IS PER TRANSLATION. The manifest is reached through a static and was
never cleared, so a second translation in the same JVM -- which the integration
tests do repeatedly -- inherited every file the previous application recorded.
Reset now happens in Parser.cleanup(), where the rest of a translation's state
is already reset.
FILES THAT WERE GENERATED AND UNRECORDED. The clean target's cn1_resources.rc /
cn1_resources_table.c / cn1_resources_data.S, the on-device-debug sidecar
cn1_debug_symbols.c, and every concatenated_<n> unit. A warning in any of them
matched no manifest entry and no path rule, so the census would refuse the whole
build rather than guess. Concatenation is not a corner: macOS translates that way
unconditionally and iOS does under ios.superfastBuild.
DIAGNOSTICS THE GRAMMAR DROPPED SILENTLY -- the worst failure a census can have,
because the gate then reports clean. A column is optional ("path:line: warning:"),
a diagnostic can name a file and no line at all (Xcode's per-project warnings),
and a fileless diagnostic WITH a tool prefix is a real driver warning rather than
a path the transport lost -- the prefix is what separates the two. That last
grammar found one genuine warning this build had been emitting unseen, an asset
catalog missing its accent colour; it is baselined rather than hidden.
KEYS THAT COULD NOT SURVIVE THEIR OWN CONTENTS. A message containing "|" -- gcc's
"suggest parentheses around arithmetic in operand of |" -- produced a six-field
row that the reader truncated and the format check then rejected. And quoted
identifiers only collapsed for the ASCII apostrophe, so gcc's and Apple's Unicode
quotes left every distinct name as its own row.
ORDERING, which was wrong in both directions. Vendored and SDK paths now precede
the manifest, because it is keyed on a bare file name and a pod shipping
Renderer.h would otherwise be reported as the iOS port's. But the companion-target
marker must come AFTER it: a watch build copies the port natives, the runtime and
the bundled SQLite into watch-src, so watch-src/IOSNative.m is still the port.
Testing the directory first reclassified 435 diagnostics in a real build as
generated code, which would have hidden port warnings behind an emitter key. The
committed order reclassifies exactly zero.
Every new shape is in the parser fixture, so none of them can regress silently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review finding, and a real weakness in the fix it was reviewing. Protecting the dispatch thread with ONE enclosing try kept the thread alive but let the first throwing reporter silence every later one: a CrashReporter that fails would take impl.handleEDTException and the application's own error handler down with it, so the application would never hear about an exception its port had already recovered from. Reporting to four places should not be all-or-nothing. Each step is now isolated from the others as well as from the loop, and a port whose handleEDTException throws is no longer read as having HANDLED the exception -- that would have swallowed it entirely rather than merely failing to report it once. Written out rather than routed through a helper taking a Runnable: this class compiles at Java 5, so that means an anonymous inner class per step, and three of them carry no outer state -- which SpotBugs reports as SIC_INNER_SHOULD_BE_STATIC_ANON, and that gate allows no findings. The first attempt produced exactly three. Two more findings with it: The probe's census is no longer published to the step summary. Redirecting stdout never stopped it: the tool appends to GITHUB_STEP_SUMMARY directly, so every successful run published a second census containing the synthetic warning the probe injects -- 142 lines of it -- which reads as a real finding to anyone looking at the summary. The variable is cleared for that invocation. pr.yml re-includes the warning gate's own files. The parser self-test runs from this workflow and nowhere else -- the legs that run the gate itself need a compiler -- and `!scripts/**` meant a change to the parser, its fixture or a baseline could merge without the one check that would have caught it ever running. The same block already re-includes retry.sh and the apt helpers for exactly this reason. EdtExceptionReportingTest covers the new invariant in both directions and is probe-verified: reverting to a single enclosing try fails it. Core suite 6,619 tests, no failures; SpotBugs 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The last two review findings on the census. Swift was excluded from the completeness set while the generated iOS project puts .swift files in Compile Sources and the sample carries one (SwiftKotlinNativeImpl.swift, recorded as port). A build that compiled every Objective-C unit and no Swift would have passed the check. Including it needed a grammar, because Swift is announced unlike anything else: the architecture comes second, and the line then carries either a "Compiling\ A.swift,\ B.swift" summary followed by the paths, or the paths alone. Taking a fixed position would have caught the first file of a batch and missed the rest -- so every .swift path on the line is harvested. Verified on the real log: coverage goes from "all 3142" to "all 3143" and still reports complete, and the fixture asserts both files off one SwiftCompile line. The other finding -- that completeness is measured against the PHONE manifest only, so a companion watch or tv target could compile nothing and still pass -- is recorded as a known limitation rather than half-fixed. Diagnostics from a companion are still attributed correctly; what is not verified is that it compiled everything it has. Closing it means staging the companion's own manifest from build-ios-app.sh and the builders, which is not a change to this tool. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66f7776d31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // recorded. The warning census would then attribute a diagnostic to a source | ||
| // belonging to a different app, or report files that are not in the project | ||
| // at all. | ||
| ByteCodeTranslator.sourceManifest.reset(); |
There was a problem hiding this comment.
Fresh evidence in this revision is that the reset was added to Parser.cleanup(), but Parser.writeOutput() unconditionally calls that method from its finally block after recording every generated source and after ByteCodeTranslator has recorded the copied port/runtime sources. The Apple handler then records only xmlvm.h before writing the manifest, so the iOS census receives a manifest missing virtually every compiled source: completeness can misleadingly cover zero expected units, and the first ordinary source warning becomes unattributed and fails the workflow. The new recordsSourceProvenanceOutsideTheSourceDirectory test also explicitly requires the generated, runtime, and port entries that this reset erases; reset once at the start of each top-level translation instead.
Useful? React with 👍 / 👎.
The manifest reset landed in Parser.cleanup(), which runs from the `finally` of Parser.writeOutput() -- BEFORE the output handler writes the manifest. So it discarded the translation's own records rather than the previous one's, and the manifest went out naming no generated files at all. CI caught it: clean-target (arm64) failed on recordsSourceProvenanceOutsideTheSourceDirectory, the test written for exactly this, naming HelloWorld.c and cn1_class_method_index.c as unrecorded. It belongs at the START of a translation, and that is also the property actually wanted: what must not leak is the PREVIOUS application's files. Moved to ByteCodeTranslator.main, which is how every caller enters, including the tests. I should have caught this locally. The test existed and I did not re-run it after adding the reset -- the translator built, the core suite passed, and neither touches this. The guard needed rewriting too, and this is the more useful half. I first added a cross-translation assertion to the integration test: translate twice, assert the second manifest does not name the first project's file. It passed with the reset REMOVED, because runTranslatorImpl hands every call a `new URLClassLoader(urls, null)` -- each translation gets its own copy of the statics, so the leak is unobservable from that harness. A test that cannot fail is worse than no test, so it is gone, with a note saying why. SourceManifestTest covers the contract where it can actually fail, and is probe-verified: making reset() a no-op fails it with "expected: <0> but was: <3>". It also pins renameGenerated, which the clean target relies on when it rewrites cn1_class_method_index.m to .c. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c237bc342
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| port|Ports/iOSPort/nativeSources/CodenameOne_GLViewController.m|-Wenum-conversion|implicit conversion from enumeration type ? (aka ?) to different enumeration type ? (aka ?)|1 instance(s) when the baseline was written; not yet triaged | ||
| port|Ports/iOSPort/nativeSources/CodenameOne_GLViewController.m|-Wenum-conversion|implicit conversion from enumeration type ? to different enumeration type ? (aka ?)|4 instance(s) when the baseline was written; not yet triaged | ||
| port|Ports/iOSPort/nativeSources/CodenameOne_GLViewController.m|-Wincompatible-pointer-types|incompatible pointer types initializing ? (aka ?) with an expression of type ? (aka ?)|2 instance(s) when the baseline was written; not yet triaged | ||
| port|Ports/iOSPort/nativeSources/CodenameOne_GLViewController.m|-Wint-conversion|incompatible pointer to integer conversion assigning to ? (aka ?) from ?|3 instance(s) when the baseline was written; not yet triaged |
There was a problem hiding this comment.
On the next cold ios-sim-debug census, this key will be stale: the three defaultDatePickerDate = nil assignments that produced these three pointer-to-integer diagnostics were all changed to 0 in CodenameOne_GLViewController.m. Because stale entries are failures, the warning gate will reject the build even though the warnings were intentionally fixed; remove or regenerate this baseline row.
Useful? React with 👍 / 👎.
Comment on lines
+135
to
+137
| port|Ports/iOSPort/nativeSources/IOSNative.m|-Wint-conversion|incompatible pointer to integer conversion initializing ? (aka ?) with an expression of type ?|1 instance(s) when the baseline was written; not yet triaged | ||
| port|Ports/iOSPort/nativeSources/IOSNative.m|-Wint-conversion|incompatible pointer to integer conversion passing ? to parameter of type ? (aka ?)|2 instance(s) when the baseline was written; not yet triaged | ||
| port|Ports/iOSPort/nativeSources/IOSNative.m|-Wint-conversion|incompatible pointer to integer conversion returning ? from a function with result type ? (aka ?)|1 instance(s) when the baseline was written; not yet triaged |
There was a problem hiding this comment.
These baseline keys no longer reproduce after the accompanying IOSNative.m changes: the JAVA_LONG initialized from NULL, the pointer passed to the Java callback, and the pointer returned from the blur function now use 0 or explicit uintptr_t/JAVA_LONG conversions. The next cold census will therefore classify these entries as stale and fail the iOS warning-gate stage; regenerate the baseline or delete the eliminated rows.
Useful? React with 👍 / 👎.
| port|Ports/iOSPort/nativeSources/UIWebViewEventDelegate.m|-Wunused-variable|unused variable ?|1 instance(s) when the baseline was written; not yet triaged | ||
| port|Ports/iOSPort/nativeSources/WebSocketImpl.m|-Wunsupported-availability-guard|@available does not guard availability here; use if (@available) instead|6 instance(s) when the baseline was written; not yet triaged | ||
| runtime|cn1_globals.m|-Wformat|format specifies type ? but the argument has type ?|2 instance(s) when the baseline was written; not yet triaged | ||
| runtime|cn1_globals.m|-Wint-conversion|incompatible pointer to integer conversion assigning to ? (aka ?) from ?|2 instance(s) when the baseline was written; not yet triaged |
There was a problem hiding this comment.
The sole implicit NSString *-to-JAVA_LONG assignment in fromNSString() now casts through uintptr_t, eliminating the diagnostic represented by this key in each staged target. On the next cold ios-sim-debug census this row will be reported as stale, causing the gate to fail despite the warning having been fixed; remove or regenerate the entry.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale is in the commit message and in code comments; this body is a pointer only.
What it does. Records where every file in a generated project came from (
cn1-source-manifest.txt), then attributes each compiler warning in a build log togenerated/runtime/port/vendored/sdk/toolchain. Ownership is not recoverable from the path -- the translator copies port natives into the same flat directory as its own output.Not gating yet. No baseline is committed. The census is report-only on
scripts-ios.yml'sbuild-iosleg (the only iOS leg that compiles cold), gated onCN1_WARNING_CENSUS, which nothing a customer runs sets.pr.ymlgains only a parser self-test that needs no compiler.Local measurement (clean target, JavaAPI-only app,
-Wall -Wextra): 3,802 warning lines from a cold build -- 3,035 in generated code across 8 kinds, 185 in the runtime. Top rows are-Wunused-parameter(1694) and-Wunused-variable(1002), both single-emitter fixes. The runtime carries several worth triaging as bugs rather than noise, including-Wpointer-to-int-cast(cast to a smaller integer type).Next step is freezing the baselines from this PR's CI run.
🤖 Generated with Claude Code