Fluff UP001: stop wrap at bare scope containing nested *= - #87
Open
manz wants to merge 2 commits into
Open
Conversation
ff4 #31 friction: `*= ADDR / opcodes / { ... *= INNER ... }` had the bare scope swallowed inside the new `.alloc at ADDR { ... }` wrap. Resulting alloc spanned ADDR..INNER (672657 bytes for the real ff4 case), allocator refused. Add `CompoundAstNode` containing nested placement directives (`*=` / `.alloc`) to the wrap boundary set. Bare scopes without nested placements stay non-terminating since they don't change the cursor. `_contains_placement` recurses into further bare scopes to catch multi-level nesting.
Deploying a816 with
|
| Latest commit: |
16beb8b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fa37d022.a816.pages.dev |
| Branch Preview URL: | https://fix-up001-bare-scope-boundar.a816.pages.dev |
ff4 #32 surfaced that the previous stop-list approach was too narrow: followed by 30 `.import`s engulfed the entire tail of the file because IfAstNode / ImportAstNode / IncludeAstNode were not in the boundary set. Switch to an allow-list. Wrap body extends only across pure emit-style nodes (opcodes, data, text, ascii, labels, constants, register-size, macro-apply, comments, docstrings). First node outside that set terminates the wrap. Separately: when the body contains `.incbin` / `.import` / `.include`, skip the conversion entirely. These rely on direct-mode chain semantics (cross-bank silent overflow for incbin; cursor-drives-module-placement for import/include) that don't translate mechanically to `.alloc at`. User migrates by hand: - `.incbin` blocks bigger than a bank: split per bank. - `.import` chains: lift imports to top of file, move placement into the modules themselves. Resolves the 672657-byte mega-wrap from ff4 #31 + the chain-engulf bug from ff4 #32. Per-PR build now safe for ff4.
|
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
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.



Summary
UP001 autofix over-extended through bare
{ ... }scopes whenthe scope contained a nested
*=. The outer wrap engulfed thescope, producing an oversized alloc that the allocator refused.
ff4 #31 friction report:
Before
UP001 produced:
The bare scope got dragged inside. Allocator size = $14F656 - $00B335.
After
Wrap closes before the bare brace. Scope keeps its independent
placement context.
Fix
_next_placement_or_endnow treatsCompoundAstNode(bare{ ... }) as a wrap boundary when it contains a nested*=or.alloc. Recurses into further bare scopes via_contains_placementso multi-level nesting is caught too.Bare scopes without placement directives stay non-terminating
since they don't reset the cursor.
Out of scope
UP001 doesn't recurse INTO the bare scope to flag the inner
*=(separate follow-up). The pre-flight check that refusesthe rewrite when it would produce a nested-
*=alloc is alsodeferred — the boundary fix now produces a correct wrap on its
own, so the pre-flight is belt-and-braces.