From 8361c66b5182c3caf7b1cbfcbc5faaa61d442f9f Mon Sep 17 00:00:00 2001 From: JulioSergioFS Date: Mon, 31 Aug 2026 18:29:49 -0300 Subject: [PATCH 1/2] feat(located): support ARRAY at a physical address `HR_myData AT %MW60 : ARRAY [0..66] OF WORD` was rejected. The reason given upstream was that MatIEC could not express a located non-elementary type (openplc-editor#565) -- but that constraint left with MatIEC, and nothing in this compiler stands in its way: the descriptor table is flat, one {area, size, index, pointer} row per slot, so an array is N rows rather than a new mechanism. Analyzer: validate the ELEMENT type against the address size class, and detect duplicates by slot-range overlap instead of address equality -- an array occupies slotCount consecutive slots, so `x AT %MW60 : ARRAY[0..66] OF WORD` collides with a plain `y AT %MW61 : WORD` even though the two addresses differ. Reject the array shapes with no linear layout (multi-dimensional, variable-length) with their own sentence, so none of them falls through to the type-compatibility error and reports the internal `__INLINE_ARRAY_` spelling at the user. Codegen: expand a located array into one descriptor per element, walking the address forward by one slot each time (bit addresses continue across the byte boundary). Also fix the pointer initialiser, which located each descriptor by variable NAME. With N descriptors sharing one name that resolved every element to slot 0: element 0 bound N times, elements 1..N-1 left null. Walk the array by position instead -- the index is right there. Verified end to end: the issue's own declaration compiles to 67 descriptors over %MW60..%MW126, all 67 bound to distinct storage, and a write through one descriptor lands in that element alone. Co-Authored-By: Claude Opus 5 (1M context) --- src/backend/codegen.ts | 217 ++++++++++++++++++----- src/semantic/analyzer.ts | 195 ++++++++++++++++---- tests/semantic/located-variables.test.ts | 157 ++++++++++++++++ 3 files changed, 492 insertions(+), 77 deletions(-) diff --git a/src/backend/codegen.ts b/src/backend/codegen.ts index 3b61fee..b7c2858 100644 --- a/src/backend/codegen.ts +++ b/src/backend/codegen.ts @@ -95,6 +95,17 @@ interface LocatedVarDescriptor { bitIndex: number; typeName: string; programName: string; + /** + * IEC index of the array element this descriptor binds, for a located + * ARRAY. Absent for a scalar. + * + * A located array is emitted as one descriptor PER ELEMENT, laid out over + * consecutive addresses -- `AT %MW60 : ARRAY [0..66] OF WORD` becomes 67 + * descriptors at %MW60..%MW126. The descriptor table is flat and carries no + * notion of an aggregate, so the element index is what tells the pointer + * initialiser to bind `arr[i]` rather than `arr` (openplc-editor#565). + */ + elementIndex?: number; } /** @@ -2688,7 +2699,14 @@ export class CodeGenerator { // (not a real program) keeps it out of every program's located_range. if (gvar.address) { this.collectLocatedVarFromModel( - { name: gvar.name, typeName: gvar.typeName, address: gvar.address }, + { + name: gvar.name, + typeName: gvar.typeName, + address: gvar.address, + // Carried through so a located ARRAY global expands to one + // descriptor per element rather than binding only its first. + arrayDimensions: gvar.arrayDimensions, + }, "@config", ); } @@ -5689,6 +5707,72 @@ export class CodeGenerator { return count; } + /** + * Push the descriptor(s) one located declaration produces. + * + * A scalar produces one. An ARRAY produces one PER ELEMENT, walking the + * address forward by one slot each time, so `AT %MW60 : ARRAY [0..66] OF + * WORD` fills %MW60..%MW126 (openplc-editor#565). The runtime table is flat + * and knows nothing about aggregates -- the expansion happens here so that + * every consumer of `locatedVars[]` (the descriptor array, the count, the + * per-program range, the located-globals list) gets the element-level view + * without any of them having to understand arrays. + * + * Bit addresses advance across the byte boundary (%IX0.7 -> %IX1.0), which + * is why the step is computed on the linearised index rather than on + * `byteIndex` alone. + * + * `dims` is `undefined` for a non-array. The semantic analyzer has already + * rejected the array shapes that cannot be laid out linearly (multi- + * dimensional, non-constant bounds), so anything reaching here with bounds + * is a single dimension with a known extent. + */ + private pushLocatedDescriptors( + varName: string, + address: string, + typeName: string, + programName: string, + dims: Array<{ start: number; end: number }> | undefined, + ): void { + const parsed = parseLocatedAddress(address); + if (!parsed) return; + + const dim = dims?.length === 1 ? dims[0] : undefined; + if (!dim) { + this.locatedVars.push({ + varName, + address, + area: parsed.area, + size: parsed.size, + byteIndex: parsed.byteIndex, + bitIndex: parsed.bitIndex, + typeName, + programName, + }); + return; + } + + const isBit = parsed.size === "Bit"; + const baseSlot = isBit + ? parsed.byteIndex * 8 + parsed.bitIndex + : parsed.byteIndex; + + for (let iecIndex = dim.start; iecIndex <= dim.end; iecIndex++) { + const slot = baseSlot + (iecIndex - dim.start); + this.locatedVars.push({ + varName, + address, + area: parsed.area, + size: parsed.size, + byteIndex: isBit ? Math.floor(slot / 8) : slot, + bitIndex: isBit ? slot % 8 : 0, + typeName, + programName, + elementIndex: iecIndex, + }); + } + } + /** * Collect a located variable for descriptor array generation. */ @@ -5699,43 +5783,41 @@ export class CodeGenerator { ): void { if (!decl.address) return; - const parsed = parseLocatedAddress(decl.address); - if (!parsed) return; - - this.locatedVars.push({ + this.pushLocatedDescriptors( varName, - address: decl.address, - area: parsed.area, - size: parsed.size, - byteIndex: parsed.byteIndex, - bitIndex: parsed.bitIndex, - typeName: decl.type.name, + decl.address, + decl.type.name, programName, - }); + // Inline `ARRAY [a..b] OF T`: the AST builder resolves the bounds onto + // the TypeReference. A named ARRAY type carries none here and is + // handled by the model path, which resolves the alias first. + decl.type.arrayDimensions, + ); } /** * Collect a located variable from project model for descriptor array generation. */ private collectLocatedVarFromModel( - decl: { name: string; typeName: string; address?: string }, + decl: { + name: string; + typeName: string; + address?: string; + // Explicit `| undefined` (not just `?`): exactOptionalPropertyTypes is + // on, and callers forward an optional field straight through. + arrayDimensions?: Array<{ start: number; end: number }> | undefined; + }, programName: string, ): void { if (!decl.address) return; - const parsed = parseLocatedAddress(decl.address); - if (!parsed) return; - - this.locatedVars.push({ - varName: decl.name, - address: decl.address, - area: parsed.area, - size: parsed.size, - byteIndex: parsed.byteIndex, - bitIndex: parsed.bitIndex, - typeName: decl.typeName, + this.pushLocatedDescriptors( + decl.name, + decl.address, + decl.typeName, programName, - }); + decl.arrayDimensions, + ); } /** @@ -5775,15 +5857,21 @@ export class CodeGenerator { this.emitHeader(" */"); this.emitHeader(""); - // Forward declarations for program instances + // Forward declarations for program instances. + // + // One line per DECLARATION, not per descriptor: a located array expands to + // one descriptor per element, and repeating the same "AT %MW60" line 67 + // times would bury the rest of the header in noise. + const listed = new Set(); for (const locVar of this.locatedVars) { const scope = locVar.programName === "@config" ? "configuration" : `Program_${locVar.programName}`; - this.emitHeader( - `// Forward: ${locVar.varName} AT ${locVar.address} in ${scope}`, - ); + const line = `// Forward: ${locVar.varName} AT ${locVar.address} in ${scope}`; + if (listed.has(line)) continue; + listed.add(line); + this.emitHeader(line); } if (isEmpty) { this.emitHeader("// (no located variables — placeholder entry only)"); @@ -5844,7 +5932,7 @@ export class CodeGenerator { const comma = i < this.locatedVars.length - 1 ? "," : ""; this.emit( ` { LocatedArea::${locVar.area}, LocatedSize::${locVar.size}, ` + - `${locVar.byteIndex}, ${locVar.bitIndex}, {0, 0, 0}, nullptr }${comma} // ${locVar.varName} AT ${locVar.address}`, + `${locVar.byteIndex}, ${locVar.bitIndex}, {0, 0, 0}, nullptr }${comma} // ${this.describeLocatedDescriptor(locVar)}`, ); } } @@ -5904,7 +5992,9 @@ export class CodeGenerator { for (let i = 0; i < globals.length; i++) { const g = globals[i]!; const comma = i < globals.length - 1 ? "," : ""; - this.emit(` nullptr${comma} // ${g.varName} AT ${g.address}`); + this.emit( + ` nullptr${comma} // ${this.describeLocatedDescriptor(g)}`, + ); } } this.emit("};"); @@ -5965,17 +6055,17 @@ export class CodeGenerator { if (progVars.length === 0) return; this.emit(`${indent}// Initialize located variable pointers`); - for (const locVar of progVars) { - // Find the index of this variable in the global array - const index = this.locatedVars.findIndex( - (v) => - v.varName === locVar.varName && v.programName === locVar.programName, + // Walk the global array by position rather than looking each entry back + // up by name. A located ARRAY contributes one descriptor per element, all + // sharing a varName, so a name lookup (`findIndex`) resolves every one of + // them to the FIRST slot: element 0 would be bound N times and elements + // 1..N-1 left null (openplc-editor#565). The index is right here anyway. + for (let index = 0; index < this.locatedVars.length; index++) { + const locVar = this.locatedVars[index]!; + if (locVar.programName !== programName) continue; + this.emit( + `${indent}locatedVars[${index}].pointer = ${this.locatedStorageExpr(locVar, memberAccess)};`, ); - if (index >= 0) { - this.emit( - `${indent}locatedVars[${index}].pointer = ${locVar.varName}${memberAccess}.raw_ptr();`, - ); - } } // Configuration VAR_GLOBALs additionally record their storage pointer in @@ -5990,13 +6080,56 @@ export class CodeGenerator { for (let g = 0; g < progVars.length; g++) { const locVar = progVars[g]!; this.emit( - `${indent}locatedGlobals[${g}] = ${locVar.varName}${memberAccess}.raw_ptr();`, + `${indent}locatedGlobals[${g}] = ${this.locatedStorageExpr(locVar, memberAccess)};`, ); } this.emit("#endif"); } } + /** + * The C++ expression yielding the storage a descriptor binds to. + * + * Scalar: `name[.value].raw_ptr()`. Array element: `name[.value][i].raw_ptr()`, + * where `i` is the IEC index — `IEC_ARRAY_1D::operator[]` maps the declared + * index range onto its internal storage, so the declared index is what goes + * in, not a zero-based offset. + */ + private locatedStorageExpr( + locVar: LocatedVarDescriptor, + memberAccess: string, + ): string { + const element = + locVar.elementIndex === undefined ? "" : `[${locVar.elementIndex}]`; + return `${locVar.varName}${memberAccess}${element}.raw_ptr()`; + } + + /** + * How a descriptor labels itself in the generated table's trailing comment. + * + * For an array element this is the address the element actually occupies, + * not the array's declared base — 67 rows all reading `AT %MW60` would tell + * a reader nothing about which slot each row binds. + */ + private describeLocatedDescriptor(locVar: LocatedVarDescriptor): string { + if (locVar.elementIndex === undefined) { + return `${locVar.varName} AT ${locVar.address}`; + } + const areaChar = { Input: "I", Output: "Q", Memory: "M" }[locVar.area]; + const sizeChar = { + Bit: "X", + Byte: "B", + Word: "W", + DWord: "D", + LWord: "L", + }[locVar.size]; + const offset = + locVar.size === "Bit" + ? `${locVar.byteIndex}.${locVar.bitIndex}` + : `${locVar.byteIndex}`; + return `${locVar.varName}[${locVar.elementIndex}] AT %${areaChar}${sizeChar}${offset}`; + } + /** * Emit a line to the implementation output. */ diff --git a/src/semantic/analyzer.ts b/src/semantic/analyzer.ts index dd29c49..927a057 100644 --- a/src/semantic/analyzer.ts +++ b/src/semantic/analyzer.ts @@ -102,7 +102,55 @@ function parseAddress(address: string): ParsedAddress | null { } /** - * Get the expected IEC types for a given address size. + * Variable-block kinds that may carry a physical location ("AT %..."). + * + * IEC 61131-3 allows located declarations in VAR and VAR_GLOBAL only — interface + * sections describe a call contract, not hardware. The editor enforces the same + * set at edit and load time (DISALLOWED_LOCATION_CLASSES, GitHub issue #904), so + * enforcing it here keeps hand-written and editor-authored ST consistent. + * + * VAR_EXTERNAL is the sharpest case: it references storage a CONFIGURATION + * VAR_GLOBAL owns, codegen emits it as `GlobalVar*` and collects located + * variables from local declarations only, so an address written there is silently + * dropped while also duplicating the address the global legitimately claims. + */ +const LOCATABLE_BLOCK_TYPES: ReadonlySet = new Set([ + "VAR", + "VAR_GLOBAL", +]); + +/** + * The bank a located address lives in: its area and its size class. + * + * Two addresses can only collide within one bank. The image is not flat memory + * -- each size class has its own array in the runtime (bool_memory[][], + * int_memory[], dint_memory[], lint_memory[]) and the index selects an element + * of THAT array -- so %MW0 and %MD0 name unrelated storage rather than + * overlapping bytes. + */ +function bankKey(parsed: ParsedAddress): string { + return `${parsed.area}${parsed.size}`; +} + +/** + * The first slot a located address names, as a linear index into its bank. + * + * Bit addresses linearise as `byte*8 + bit` so that consecutive bits are + * consecutive slots across a byte boundary (%IX0.7 and %IX1.0 are slots 7 and + * 8). Every other size class indexes its array directly. + */ +function firstSlot(parsed: ParsedAddress): number { + return parsed.size === "X" + ? parsed.byteIndex * 8 + parsed.bitIndex + : parsed.byteIndex; +} + +/** + * Elementary types that may sit at an address of the given size. + * + * For an array, this is checked against the ELEMENT type: `ARRAY [0..66] OF + * WORD AT %MW60` occupies 67 consecutive WORD slots, so what has to fit the + * `W` size class is WORD, not the array as a whole. */ function getCompatibleTypes(size: "X" | "B" | "W" | "D" | "L"): string[] { switch (size) { @@ -120,34 +168,72 @@ function getCompatibleTypes(size: "X" | "B" | "W" | "D" | "L"): string[] { } /** - * Variable-block kinds that may carry a physical location ("AT %..."). + * What a located declaration actually occupies in the process image. * - * IEC 61131-3 allows located declarations in VAR and VAR_GLOBAL only — interface - * sections describe a call contract, not hardware. The editor enforces the same - * set at edit and load time (DISALLOWED_LOCATION_CLASSES, GitHub issue #904), so - * enforcing it here keeps hand-written and editor-authored ST consistent. + * A plain variable takes one slot and must itself fit the size class. An array + * takes one slot PER ELEMENT, laid out consecutively from the declared address, + * and it is the element type that must fit -- `HR AT %MW60 : ARRAY [0..66] OF + * WORD` means %MW60 through %MW126, each a WORD (openplc-editor#565). * - * VAR_EXTERNAL is the sharpest case: it references storage a CONFIGURATION - * VAR_GLOBAL owns, codegen emits it as `GlobalVar*` and collects located - * variables from local declarations only, so an address written there is silently - * dropped while also duplicating the address the global legitimately claims. - */ -const LOCATABLE_BLOCK_TYPES: ReadonlySet = new Set([ - "VAR", - "VAR_GLOBAL", -]); - -/** - * Create a canonical address key for duplicate detection. + * Arrays are supported here because nothing in the descriptor table stands in + * the way: it is flat, one `{area, size, index, pointer}` row per slot, so an + * array is N rows rather than a new mechanism. (The pre-strucpp toolchain + * refused these because MatIEC could not express them at all; that constraint + * left with MatIEC.) * - * Exact match is the right test: the image is not flat memory. Each size class - * has its own array in the runtime (bool_memory[][], int_memory[], dint_memory[], - * lint_memory[]) and byte_index indexes that array, so %MW0 and %MD0 name - * unrelated storage rather than overlapping bytes. Two declarations collide only - * when area, size, byte and bit all match. + * Returns a `reason` instead of a shape for the array forms that have no + * meaningful linear layout. Each is rejected with its own sentence rather than + * falling through to the type-compatibility error, which would otherwise + * report the compiler's internal `__INLINE_ARRAY_` spelling at the user. */ -function addressKey(parsed: ParsedAddress): string { - return `${parsed.area}${parsed.size}${parsed.byteIndex}.${parsed.bitIndex}`; +type LocatedShape = + | { elementTypeName: string; slotCount: number; reason?: undefined } + | { reason: string; elementTypeName?: undefined; slotCount?: undefined }; + +function resolveLocatedShape( + type: TypeReference, + ast: CompilationUnit, +): LocatedShape { + // A variable-length array carries no bounds anywhere — the AST builder + // records only its rank, in the synthetic `__VLA__` name — so + // `resolveArrayShape` cannot see it and it would otherwise fall through to + // the scalar branch and be reported as an incompatible type named + // `__VLA_1D_WORD`. Catch it here so the message says what is actually wrong. + if (type.name.toUpperCase().startsWith("__VLA_")) { + return { + reason: `its length is not known at compile time. A located array needs constant bounds, because each element is bound to a fixed address before the program runs`, + }; + } + + const shape: ArrayShape | undefined = resolveArrayShape(type, ast); + if (!shape) { + // Not an array: the declaration is the slot, and its own type is what + // has to fit the size class. + return { elementTypeName: type.name, slotCount: 1 }; + } + + if (shape.dims.length !== 1) { + return { + reason: `a ${shape.dims.length}-dimensional array has no single linear run of addresses to occupy. Declare it unlocated, or use a one-dimensional array`, + }; + } + + const dim = shape.dims[0]; + if (!dim) { + // `ARRAY [*]` or a bound that isn't a compile-time constant. The runtime + // binds each element to a fixed address, so the count has to be known + // when the descriptor table is emitted -- not when the program runs. + return { + reason: `its length is not known at compile time. A located array needs constant bounds, because each element is bound to a fixed address before the program runs`, + }; + } + + const slotCount = arrayDimSize(dim); + if (slotCount === undefined || slotCount <= 0) { + return { reason: `its declared bounds are empty` }; + } + + return { elementTypeName: shape.elementTypeName, slotCount }; } // ============================================================================= @@ -1279,7 +1365,11 @@ export class SemanticAnalyzer { * - Bit index must be 0-7 for bit addresses */ private validateLocatedVariables(ast: CompilationUnit): void { - const addressMap = new Map(); + /** Slot ranges already claimed, keyed by bank (`area + size`). */ + const claimedSlots = new Map< + string, + Array<{ start: number; end: number; owner: LocatedVarInfo }> + >(); const instanceCounts = this.countProgramInstantiations(ast); // Configuration globals participate in every rule below, above all in the @@ -1329,11 +1419,30 @@ export class SemanticAnalyzer { } } - // Rule 2: Validate type compatibility with address size + // Rule 2: the type must fit the address size, and (for an array) the + // array must have a linear run of addresses to occupy at all. + const shape = resolveLocatedShape(decl.type, ast); + if (shape.reason !== undefined) { + this.addError( + `Located variable '${locVar.name}' at ${locVar.address} cannot be placed: ${shape.reason}.`, + decl.sourceSpan.startLine, + decl.sourceSpan.startCol, + decl.sourceSpan.file, + ); + continue; + } + const compatibleTypes = getCompatibleTypes(locVar.parsed.size); - if (!compatibleTypes.includes(locVar.typeName.toUpperCase())) { + if (!compatibleTypes.includes(shape.elementTypeName.toUpperCase())) { + // For an array the mismatch is in the ELEMENT type, so say so — + // "Type 'ARRAY [0..66] OF STRING'" would point at the wrong half of + // the declaration. + const subject = + shape.slotCount > 1 + ? `Array element type '${shape.elementTypeName}'` + : `Type '${shape.elementTypeName}'`; this.addError( - `Type '${locVar.typeName}' is not compatible with address size '${locVar.parsed.size}' in '${locVar.address}'. Expected one of: ${compatibleTypes.join(", ")}`, + `${subject} is not compatible with address size '${locVar.parsed.size}' in '${locVar.address}'. Expected one of: ${compatibleTypes.join(", ")}`, decl.sourceSpan.startLine, decl.sourceSpan.startCol, decl.sourceSpan.file, @@ -1353,18 +1462,34 @@ export class SemanticAnalyzer { ); } - // Rule 4: Check for duplicate addresses - const key = addressKey(locVar.parsed); - const existing = addressMap.get(key); - if (existing) { + // Rule 4: no two declarations may claim the same slot. + // + // Overlap, not equality: an array occupies `slotCount` consecutive + // slots, so `x AT %MW60 : ARRAY [0..66] OF WORD` collides with a plain + // `y AT %MW61 : WORD` even though the two addresses differ. Comparing + // addresses for equality (which is all that was needed while every + // declaration took exactly one slot) would let the second variable + // silently share storage with an element of the first. + const bank = bankKey(locVar.parsed); + const start = firstSlot(locVar.parsed); + const end = start + shape.slotCount - 1; + + const claimsInBank = claimedSlots.get(bank) ?? []; + const clash = claimsInBank.find((c) => start <= c.end && c.start <= end); + if (clash) { this.addError( - `Duplicate address ${locVar.address}: variable '${locVar.name}' conflicts with '${existing.name}'`, + `Duplicate address ${locVar.address}: variable '${locVar.name}' conflicts with '${clash.owner.name}'${ + clash.end > clash.start || end > start + ? ` (${clash.owner.name} occupies ${clash.owner.address} onwards)` + : "" + }`, decl.sourceSpan.startLine, decl.sourceSpan.startCol, decl.sourceSpan.file, ); } else { - addressMap.set(key, locVar); + claimsInBank.push({ start, end, owner: locVar }); + claimedSlots.set(bank, claimsInBank); } } } diff --git a/tests/semantic/located-variables.test.ts b/tests/semantic/located-variables.test.ts index 33202a1..ec5b8f2 100644 --- a/tests/semantic/located-variables.test.ts +++ b/tests/semantic/located-variables.test.ts @@ -801,6 +801,163 @@ describe('Phase 2.3 - Located Variables', () => { }); }); + // A located ARRAY occupies one slot PER ELEMENT, laid out consecutively from + // the declared address. The pre-strucpp toolchain refused these outright + // because MatIEC could not express a located non-elementary type + // (openplc-editor#565); the descriptor table here is flat, so an array is + // N rows rather than a new mechanism. + describe('Semantic: Located Arrays', () => { + it('accepts an array of an elementary type compatible with the address size', () => { + const source = ` + PROGRAM Main + VAR buffer AT %MW60 : ARRAY [0..66] OF WORD; END_VAR + END_PROGRAM + `; + expect(compile(source).success).toBe(true); + }); + + it('accepts the same shape spelled as a named ARRAY type', () => { + const source = ` + TYPE Buf : ARRAY [0..9] OF WORD; END_TYPE + PROGRAM Main + VAR buffer AT %MW0 : Buf; END_VAR + END_PROGRAM + `; + expect(compile(source).success).toBe(true); + }); + + it('validates the ELEMENT type against the address size, not the array', () => { + const source = ` + PROGRAM Main + VAR buffer AT %MW0 : ARRAY [0..3] OF BOOL; END_VAR + END_PROGRAM + `; + const result = compile(source); + expect(result.success).toBe(false); + // The message must name the element type, not the compiler's internal + // __INLINE_ARRAY_ spelling, which is what leaked before. + expect(result.errors.some(e => e.message.includes("Array element type 'BOOL'"))).toBe(true); + expect(result.errors.every(e => !e.message.includes('__INLINE_ARRAY'))).toBe(true); + }); + + it.each([ + ['multi-dimensional', 'ARRAY [0..3, 0..3] OF WORD', 'no single linear run'], + ['variable-length', 'ARRAY [*] OF WORD', 'not known at compile time'], + ])('rejects a %s located array with its own message', (_label, type, fragment) => { + const source = ` + PROGRAM Main + VAR buffer AT %MW0 : ${type}; END_VAR + END_PROGRAM + `; + const result = compile(source); + expect(result.success).toBe(false); + expect(result.errors.some(e => e.message.includes(fragment))).toBe(true); + expect(result.errors.every(e => !e.message.includes('__INLINE_ARRAY'))).toBe(true); + }); + + it('detects a collision with a scalar that lands inside the array', () => { + // %MW1 is the array's second element. Comparing addresses for equality + // — all that was needed while every declaration took one slot — would + // let these two silently share storage. + const source = ` + PROGRAM Main + VAR + buffer AT %MW0 : ARRAY [0..3] OF WORD; + other AT %MW1 : WORD; + END_VAR + END_PROGRAM + `; + const result = compile(source); + expect(result.success).toBe(false); + expect(result.errors.some(e => e.message.includes('Duplicate address'))).toBe(true); + }); + + it('allows a scalar immediately after the array ends', () => { + const source = ` + PROGRAM Main + VAR + buffer AT %MW0 : ARRAY [0..3] OF WORD; + other AT %MW4 : WORD; + END_VAR + END_PROGRAM + `; + expect(compile(source).success).toBe(true); + }); + + it('does not collide with the same index in a different size class', () => { + // %MW0 and %MD0 index different runtime arrays — the image is not flat + // memory, and an array must not make it look like one. + const source = ` + PROGRAM Main + VAR + words AT %MW0 : ARRAY [0..3] OF WORD; + dwords AT %MD0 : ARRAY [0..3] OF DINT; + END_VAR + END_PROGRAM + `; + expect(compile(source).success).toBe(true); + }); + }); + + describe('Code Generation: Located Arrays', () => { + const arrayProgram = ` + PROGRAM Main + VAR buffer AT %MW60 : ARRAY [0..66] OF WORD; END_VAR + END_PROGRAM + `; + + it('emits one descriptor per element, over consecutive addresses', () => { + const result = compile(arrayProgram); + expect(result.success).toBe(true); + expect(result.headerCode).toContain('locatedVarsCount = 67'); + expect(result.cppCode).toContain('LocatedVar locatedVars[67]'); + // First and last element land on the ends of the declared run. + expect(result.cppCode).toContain('LocatedSize::Word, 60, 0'); + expect(result.cppCode).toContain('LocatedSize::Word, 126, 0'); + }); + + it('binds every element to its own storage', () => { + // The pointer initialiser used to find its descriptor by variable NAME. + // With N descriptors sharing one name that resolved every element to + // slot 0: element 0 bound N times, elements 1..N-1 left null. + const result = compile(arrayProgram); + const inits = result.cppCode.match(/locatedVars\[\d+\]\.pointer = [^;]+;/g) ?? []; + expect(inits).toHaveLength(67); + expect(new Set(inits).size).toBe(67); + expect(result.cppCode).toContain('locatedVars[0].pointer = BUFFER[0].raw_ptr();'); + expect(result.cppCode).toContain('locatedVars[66].pointer = BUFFER[66].raw_ptr();'); + }); + + it('labels each descriptor with the address that element occupies', () => { + const result = compile(arrayProgram); + expect(result.cppCode).toContain('// BUFFER[0] AT %MW60'); + expect(result.cppCode).toContain('// BUFFER[66] AT %MW126'); + }); + + it('walks a bit array across the byte boundary', () => { + const source = ` + PROGRAM Main + VAR flags AT %QX0.6 : ARRAY [0..3] OF BOOL; END_VAR + END_PROGRAM + `; + const result = compile(source); + expect(result.success).toBe(true); + // %QX0.6, %QX0.7, %QX1.0, %QX1.1 — the run continues into the next byte. + expect(result.cppCode).toContain('LocatedSize::Bit, 0, 6'); + expect(result.cppCode).toContain('LocatedSize::Bit, 0, 7'); + expect(result.cppCode).toContain('LocatedSize::Bit, 1, 0'); + expect(result.cppCode).toContain('LocatedSize::Bit, 1, 1'); + }); + + it('lists a located array once in the forward-declaration comments', () => { + // One line per declaration, not per descriptor — 67 identical lines + // would bury the rest of the header. + const result = compile(arrayProgram); + const forwards = result.headerCode.match(/\/\/ Forward: BUFFER AT %MW60/g) ?? []; + expect(forwards).toHaveLength(1); + }); + }); + // locatedGlobals[] states which locatedVars[] entries are CONFIGURATION // VAR_GLOBAL ... AT. Without it a host runtime has to infer the split, and // inferring it from array position is what broke every located global as soon From b2d5a1aef0be159dcc9f5573847040ad14fb272d Mon Sep 17 00:00:00 2001 From: JulioSergioFS Date: Wed, 2 Sep 2026 22:11:09 -0300 Subject: [PATCH 2/2] fix(located): resolve named ARRAY types in codegen, not just inline ones Regression found in review. The AST builder writes bounds onto the TypeReference only for an INLINE `ARRAY [a..b] OF T`; a named type carries none. `collectLocatedVar` read `decl.type.arrayDimensions` directly, while the analyzer's `resolveLocatedShape` resolves named types through `resolveArrayShapeByName` -- so the two passes disagreed: TYPE Buf : ARRAY [0..9] OF WORD; END_TYPE VAR_GLOBAL HR AT %MW60 : Buf; END_VAR Semantics accepted it and correctly reserved ten slots (a second variable at %MW65 was properly rejected), then codegen emitted ONE descriptor and called `.raw_ptr()` on the array itself: error: no member named 'raw_ptr' in 'IEC_ARRAY_1D, ArrayBounds<0, 9>>' Worse than the state before this branch, where the analyzer refused the declaration cleanly -- it failed in the C++ backend leaking internal type names, which is the failure mode this branch set out to remove. Codegen now resolves the shape by name when the declaration carries no bounds, mirroring what the analyzer already does. The existing test missed this because it asserted only `compile(source).success` with no CONFIGURATION, so codegen's located path never ran. Replaced with one that emits the path and checks what comes out: ten descriptors over %MW60-%MW69, ten distinct pointers, and no `.raw_ptr()` on the aggregate. Verified the generated C++ now passes `g++ -fsyntax-only`. Co-Authored-By: Claude Opus 5 (1M context) --- src/backend/codegen.ts | 33 ++++++++++++++++++++++- tests/semantic/located-variables.test.ts | 34 ++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/backend/codegen.ts b/src/backend/codegen.ts index b7c2858..71c4c7b 100644 --- a/src/backend/codegen.ts +++ b/src/backend/codegen.ts @@ -69,6 +69,7 @@ import { isImplicitlyConvertible, resolveFieldType as resolveFieldTypeUtil, resolveArrayElementType as resolveArrayElementTypeUtil, + resolveArrayShapeByName, typeName as typeNameUtil, buildEnumMemberMap, type EnumMemberEntry, @@ -5727,6 +5728,35 @@ export class CodeGenerator { * dimensional, non-constant bounds), so anything reaching here with bounds * is a single dimension with a known extent. */ + /** + * The dimensions a located declaration actually has, resolving a NAMED + * ARRAY type the way the semantic analyzer already does. + * + * The AST builder writes bounds onto the TypeReference only for an INLINE + * `ARRAY [a..b] OF T`. A named type (`TYPE Buf : ARRAY [0..9] OF WORD`) has + * none, so reading `decl.type.arrayDimensions` alone made codegen see a + * scalar where the analyzer had seen ten slots: one descriptor was emitted + * and the pointer initialiser called `.raw_ptr()` on the array itself, which + * does not compile. Resolving by name here keeps the two passes agreeing. + * + * Answers `undefined` for anything that is not a single fixed dimension -- + * multi-dimensional and variable-length shapes are rejected in semantics and + * never reach codegen, so this is a fallback rather than a second opinion. + */ + private resolveLocatedDims( + typeName: string, + dims: Array<{ start: number; end: number }> | undefined, + ): Array<{ start: number; end: number }> | undefined { + if (dims && dims.length > 0) return dims; + if (!this.ast) return undefined; + + const shape = resolveArrayShapeByName(typeName, this.ast); + if (!shape || shape.dims.length !== 1) return undefined; + + const dim = shape.dims[0]; + return dim ? [dim] : undefined; + } + private pushLocatedDescriptors( varName: string, address: string, @@ -5737,7 +5767,8 @@ export class CodeGenerator { const parsed = parseLocatedAddress(address); if (!parsed) return; - const dim = dims?.length === 1 ? dims[0] : undefined; + const resolved = this.resolveLocatedDims(typeName, dims); + const dim = resolved?.length === 1 ? resolved[0] : undefined; if (!dim) { this.locatedVars.push({ varName, diff --git a/tests/semantic/located-variables.test.ts b/tests/semantic/located-variables.test.ts index ec5b8f2..afd761d 100644 --- a/tests/semantic/located-variables.test.ts +++ b/tests/semantic/located-variables.test.ts @@ -826,6 +826,40 @@ describe('Phase 2.3 - Located Variables', () => { expect(compile(source).success).toBe(true); }); + it('emits per-element descriptors for a named ARRAY type, like an inline one', () => { + // A named type carries no bounds on its TypeReference, so codegen used to + // see a scalar where semantics had seen ten slots: one descriptor, and a + // pointer initialiser calling .raw_ptr() on the array itself, which does + // not compile. The CONFIGURATION is what makes codegen emit the located + // path at all -- without it the assertion above passes while the bug + // sits untouched. + const source = ` + TYPE Buf : ARRAY [0..9] OF WORD; END_TYPE + PROGRAM Main + VAR_EXTERNAL HR : Buf; END_VAR + HR[0] := 1; + END_PROGRAM + + CONFIGURATION Config0 + VAR_GLOBAL HR AT %MW60 : Buf; END_VAR + RESOURCE Res0 ON PLC + TASK task0(INTERVAL := T#20ms, PRIORITY := 0); + PROGRAM instance0 WITH task0 : Main; + END_RESOURCE + END_CONFIGURATION + `; + const result = compile(source); + expect(result.success).toBe(true); + expect(result.headerCode).toContain('locatedVarsCount = 10'); + expect(result.cppCode).toContain('LocatedSize::Word, 60, 0'); + expect(result.cppCode).toContain('LocatedSize::Word, 69, 0'); + // Each element binds its own storage; never `.raw_ptr()` on the array. + const inits = result.cppCode.match(/locatedVars\[\d+\]\.pointer = [^;]+;/g) ?? []; + expect(inits).toHaveLength(10); + expect(new Set(inits).size).toBe(10); + expect(result.cppCode).not.toMatch(/pointer = HR\.value\.raw_ptr\(\)/); + }); + it('validates the ELEMENT type against the address size, not the array', () => { const source = ` PROGRAM Main