From c5ccc7fbc197f454875b1b932ea156b409787ec6 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Mon, 3 Aug 2026 16:33:16 -0400 Subject: [PATCH 1/2] feat(codegen): emit locatedGlobals[] so runtimes stop inferring scope locatedVars[] mixes two ownership classes: POU-local `VAR ... AT`, serviced by the owning IEC task around run(), and CONFIGURATION `VAR_GLOBAL ... AT`, owned by no task and therefore serviced by a host dispatcher at a quiescent frame boundary. The descriptors carry no scope, and a runtime cannot recover it -- globals are file-scope GlobalVar singletons that appear nowhere in the configuration object graph reachable via ConfigurationInstance. So runtimes inferred the split from array position. The OpenPLC v4 runtime assumed [program-local ... ][config globals ... ] and took the tail not covered by any program's located_range() as the globals. We emit config globals FIRST (emitFileScopeGlobals runs before the program headers), so the program-local block is always the tail: one POU-local located variable made the computed count collapse to zero and silently stopped servicing EVERY located global -- %MX coils written over Modbus never reached the program, and %IX/%QX/%MW globals stopped being read and committed. Reported on the forum as "%MX locations now invalid - Runtime v4". State the scope instead of leaving it to be guessed. locatedGlobals[] holds the canonical storage pointer of each located VAR_GLOBAL -- the same raw_ptr() value written into locatedVars[].pointer -- so a runtime resolves the config-scope entries by pointer identity, with no dependence on array order. Located variables are always elementary types, so every entry is a scalar IECVar and raw_ptr() is available. Populated in the configuration constructor because raw_ptr() is not a constant expression, and to keep it beside the locatedVars[] population it must agree with. The extern "C" accessors are emitted here rather than left to a host runtime's shim. That is what makes the change backwards-compatible in both directions: the OpenPLC runtime rebuilds its shim against each upload's generated header, so a shim referencing these symbols would fail to COMPILE against an older project, whereas an accessor emitted beside its own array always matches. An older runtime never resolves the symbols and is bit-for-bit unaffected; a newer runtime probes with dlsym and skips located globals when they are absent. Guarded by STRUCPP_THREADED throughout: freestanding targets bind every located variable directly and have no dispatcher, so they pay nothing -- verified that a non-threaded build emits no locatedGlobals symbol at all. Co-Authored-By: Claude Opus 5 (1M context) --- src/backend/codegen.ts | 103 ++++++++++++++++++++++ tests/semantic/located-variables.test.ts | 105 +++++++++++++++++++++++ 2 files changed, 208 insertions(+) diff --git a/src/backend/codegen.ts b/src/backend/codegen.ts index bcc9ff28..245e5e95 100644 --- a/src/backend/codegen.ts +++ b/src/backend/codegen.ts @@ -5868,6 +5868,19 @@ export class CodeGenerator { `constexpr uint32_t locatedVarsCount = ${this.locatedVars.length};`, ); this.emitHeader(""); + + // Located CONFIGURATION VAR_GLOBALs — see generateLocatedGlobalsDefinition() + // for why the scope has to be stated rather than inferred by the runtime. + const globalCount = this.locatedVars.filter( + (v) => v.programName === "@config", + ).length; + this.emitHeader("#ifdef STRUCPP_THREADED"); + this.emitHeader( + `extern void *locatedGlobals[${globalCount === 0 ? 1 : globalCount}];`, + ); + this.emitHeader(`constexpr uint32_t locatedGlobalsCount = ${globalCount};`); + this.emitHeader("#endif"); + this.emitHeader(""); } /** @@ -5909,6 +5922,78 @@ export class CodeGenerator { this.emit("};"); this.emit(""); + + this.generateLocatedGlobalsDefinition(); + } + + /** + * Emit the located-globals pointer array plus its C-linkage accessors. + * + * `locatedVars[]` mixes two ownership classes: POU-local `VAR ... AT` (serviced + * by the owning IEC task around run()) and CONFIGURATION `VAR_GLOBAL ... AT` + * (owned by no task, so a host dispatcher must copy them at a quiescent frame + * boundary). The descriptors carry no scope, and a runtime cannot recover it: + * globals are file-scope singletons that appear nowhere in the configuration + * object graph. Runtimes previously *inferred* the split from each entry's + * position in locatedVars[], which broke as soon as a POU declared a located + * variable and silently stopped servicing every located global. + * + * So state it here instead. locatedGlobals[] holds the canonical storage + * pointer of every located VAR_GLOBAL — the same raw_ptr() value written into + * locatedVars[].pointer — letting a runtime identify the config-scope entries by + * pointer identity, independent of array order. Located variables are always + * elementary types, so every entry is a scalar IECVar and raw_ptr() exists. + * + * Emitted only under STRUCPP_THREADED: freestanding targets bind every located + * variable directly and have no dispatcher, so they would pay RAM for nothing. + * + * The accessors are emitted here rather than left to a host runtime's shim so + * the generated code stays self-contained. A shim referencing these symbols + * would fail to COMPILE against an older project's generated header, whereas an + * accessor emitted beside its own array always matches. A runtime predating + * this simply never resolves the symbols and skips located globals. + */ + private generateLocatedGlobalsDefinition(): void { + const globals = this.locatedVars.filter((v) => v.programName === "@config"); + // C++ forbids zero-length arrays at namespace scope; emit a placeholder and + // report count 0, exactly as locatedVars[] does. That lets a runtime tell + // "accessors absent" (older project — cannot service globals) apart from + // "present but count 0" (project genuinely has no located globals). + const arrayLen = globals.length === 0 ? 1 : globals.length; + + this.emit("#ifdef STRUCPP_THREADED"); + this.emit( + "// Canonical storage pointers of the located CONFIGURATION VAR_GLOBALs.", + ); + this.emit( + "// Populated in the configuration constructor (like locatedVars[] above).", + ); + this.emit(`void *locatedGlobals[${arrayLen}] = {`); + if (globals.length === 0) { + this.emit(` nullptr // placeholder; locatedGlobalsCount is 0`); + } else { + 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("};"); + this.emit(""); + this.emit( + "// C linkage: a host runtime is built once and loads many .so files, so it", + ); + this.emit( + "// cannot reach namespaced C++ symbols by mangled name portably.", + ); + this.emit( + 'extern "C" void *const *strucpp_get_located_globals(void) { return locatedGlobals; }', + ); + this.emit( + 'extern "C" uint32_t strucpp_get_located_global_count(void) { return locatedGlobalsCount; }', + ); + this.emit("#endif // STRUCPP_THREADED"); + this.emit(""); } /** @@ -5963,6 +6048,24 @@ export class CodeGenerator { ); } } + + // Configuration VAR_GLOBALs additionally record their storage pointer in + // locatedGlobals[], which is what lets a host runtime tell config-scope + // entries from POU-local ones without guessing from array position. Emitted + // here (rather than as a static initializer) because raw_ptr() is not a + // constant expression, and to keep it beside the locatedVars[] population it + // must agree with. + if (programName === "@config") { + this.emit("#ifdef STRUCPP_THREADED"); + this.emit(`${indent}// Initialize located-global pointers`); + for (let g = 0; g < progVars.length; g++) { + const locVar = progVars[g]!; + this.emit( + `${indent}locatedGlobals[${g}] = ${locVar.varName}${memberAccess}.raw_ptr();`, + ); + } + this.emit("#endif"); + } } /** diff --git a/tests/semantic/located-variables.test.ts b/tests/semantic/located-variables.test.ts index cf1b6354..91046b51 100644 --- a/tests/semantic/located-variables.test.ts +++ b/tests/semantic/located-variables.test.ts @@ -518,4 +518,109 @@ describe('Phase 2.3 - Located Variables', () => { expect(result.cppCode).toMatch(/placeholder.*locatedVarsCount is 0/); }); }); + + // 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 + // as a POU declared a located variable (forum: "%MX locations now invalid"). + describe('Code Generation: Located Globals Array', () => { + const mixedProject = ` + PROGRAM Main + VAR_EXTERNAL + gWord : DINT; + gBit : BOOL; + END_VAR + VAR localBit AT %IX0.1 : BOOL; END_VAR + localBit := gBit; + END_PROGRAM + + CONFIGURATION Config0 + VAR_GLOBAL + gWord AT %MD0 : DINT; + gBit AT %MX0.0 : BOOL; + END_VAR + RESOURCE Res0 ON PLC + TASK t(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM p WITH t : Main; + END_RESOURCE + END_CONFIGURATION + `; + + it('declares locatedGlobals with only the config-scope located vars', () => { + const result = compile(mixedProject); + expect(result.success).toBe(true); + // 3 located vars total, but only the 2 globals belong in locatedGlobals. + expect(result.headerCode).toContain('constexpr uint32_t locatedVarsCount = 3;'); + expect(result.headerCode).toContain('extern void *locatedGlobals[2];'); + expect(result.headerCode).toContain('constexpr uint32_t locatedGlobalsCount = 2;'); + }); + + it('defines the array and exports C-linkage accessors', () => { + const result = compile(mixedProject); + expect(result.cppCode).toContain('void *locatedGlobals[2] = {'); + expect(result.cppCode).toContain( + 'extern "C" void *const *strucpp_get_located_globals(void)', + ); + expect(result.cppCode).toContain( + 'extern "C" uint32_t strucpp_get_located_global_count(void)', + ); + }); + + it('populates locatedGlobals in the configuration constructor', () => { + const result = compile(mixedProject); + // Same raw_ptr() value written into locatedVars[].pointer, so a runtime can + // identify the config-scope entries by pointer identity. + expect(result.cppCode).toContain('locatedGlobals[0] = GWORD.value.raw_ptr();'); + expect(result.cppCode).toContain('locatedGlobals[1] = GBIT.value.raw_ptr();'); + }); + + it('excludes POU-local located variables', () => { + const result = compile(mixedProject); + // LOCALBIT is located but program-owned: it must never enter locatedGlobals. + expect(result.cppCode).toContain('locatedVars[2].pointer = LOCALBIT.raw_ptr();'); + expect(result.cppCode).not.toMatch(/locatedGlobals\[\d+\] = LOCALBIT/); + }); + + it('guards array, accessors and population behind STRUCPP_THREADED', () => { + // Freestanding targets bind every located variable directly and have no + // dispatcher, so they must pay nothing for this. + const result = compile(mixedProject); + expect(result.headerCode).toMatch( + /#ifdef STRUCPP_THREADED\s*\nextern void \*locatedGlobals\[2\];/, + ); + expect(result.cppCode).toMatch( + /#ifdef STRUCPP_THREADED\s*\n\/\/ Canonical storage pointers/, + ); + expect(result.cppCode).toMatch( + /#ifdef STRUCPP_THREADED\s*\n\s*\/\/ Initialize located-global pointers/, + ); + }); + + it('emits a placeholder and count 0 when no global is located', () => { + // A runtime must be able to tell "accessors absent" (older project) from + // "present but count 0" (no located globals), so the accessors are still + // emitted in this case. + const source = ` + PROGRAM Main + VAR localBit AT %IX0.1 : BOOL; END_VAR + localBit := FALSE; + END_PROGRAM + + CONFIGURATION Config0 + RESOURCE Res0 ON PLC + TASK t(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM p WITH t : Main; + END_RESOURCE + END_CONFIGURATION + `; + const result = compile(source); + expect(result.success).toBe(true); + expect(result.headerCode).toContain('constexpr uint32_t locatedGlobalsCount = 0;'); + expect(result.headerCode).toContain('extern void *locatedGlobals[1];'); + expect(result.cppCode).toMatch(/placeholder.*locatedGlobalsCount is 0/); + expect(result.cppCode).toContain( + 'extern "C" uint32_t strucpp_get_located_global_count(void)', + ); + }); + }); }); From cf59629351dbada77e161443b8c3734c97c1fde3 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Tue, 4 Aug 2026 14:26:50 -0400 Subject: [PATCH 2/2] fix(semantic): close three gaps in located-address validation validateLocatedVariables() rejected duplicate addresses across POUs, but three configurations slipped through and each produced silent misbehaviour in the runtime rather than a diagnostic. 1. Configuration VAR_GLOBALs were invisible to every located-variable rule. They live in ast.configurations[].varBlocks, while buildVarBlockSymbols only walks POU scopes (program / function / functionBlock) via ast.programs et al, so a located global was never recorded. A POU-local `VAR ... AT %MX0.0` alongside a `VAR_GLOBAL ... AT %MX0.0` therefore compiled, and the two entries resolve to one image slot -- serviced by two different paths, the owning task and the dispatcher at the quiescent frame boundary. Collect them (deduped by name, since codegen emits one file-scope singleton per name and a global repeated across configurations is one canonical global) so they take part in the duplicate, type-compatibility and bit-range rules. 2. A located variable in a PROGRAM instantiated more than once was accepted. Codegen allocates one locatedVars[] slot per *declaration*, and the pointer is assigned in the POU constructor `this`-relative -- the class cannot name its own instance -- so the last instance constructed wins and the other instances' copies are never serviced. Verified: two instances of one POU report the same located_range() and share one slot, leaving one instance's %IX3.0 orphaned. Same reasoning as the existing FUNCTION_BLOCK restriction: a physical address belongs to one point of hardware. IEC 61131-3 permits multiple program instances and answers per-instance addressing with a partly specified location (`AT %I*`) resolved by VAR_CONFIG, which is not supported here, so the fully specified form must be rejected. 3. Only VAR and VAR_GLOBAL may now carry a location. Interface sections describe a call contract, not hardware, and a located interface declaration hands the runtime two owners for one image slot -- a hazard invisible from the generated code. VAR_EXTERNAL is the sharpest case: it references storage a VAR_GLOBAL owns, codegen emits it as `GlobalVar*` and collects located vars from local declarations only, so the address is silently dropped while duplicating the global's claim. Matches the editor, which rejects the same set at edit and load time (DISALLOWED_LOCATION_CLASSES, GitHub issue #904). tests/fixtures/smart-traffic-light declared its located I/O in VAR_INPUT / VAR_OUTPUT; moved to VAR, which is where it belonged. addressKey() is deliberately left matching on exact (area, size, byte, bit). 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. Documented on the function so it is not "fixed" into a range check later. Co-Authored-By: Claude Opus 5 (1M context) --- src/semantic/analyzer.ts | 169 ++++++++++- .../src/Programs/IntersectionController.st | 8 +- tests/integration/compile.test.ts | 5 +- tests/semantic/located-variables.test.ts | 282 ++++++++++++++++++ 4 files changed, 453 insertions(+), 11 deletions(-) diff --git a/src/semantic/analyzer.ts b/src/semantic/analyzer.ts index ee2dc425..930db1e4 100644 --- a/src/semantic/analyzer.ts +++ b/src/semantic/analyzer.ts @@ -101,8 +101,32 @@ function getCompatibleTypes(size: "X" | "B" | "W" | "D" | "L"): string[] { } } +/** + * 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", +]); + /** * Create a canonical address key for duplicate detection. + * + * 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. */ function addressKey(parsed: ParsedAddress): string { return `${parsed.area}${parsed.size}${parsed.byteIndex}.${parsed.bitIndex}`; @@ -149,7 +173,11 @@ interface LocatedVarInfo { address: string; parsed: ParsedAddress; typeName: string; - scopeType: "program" | "function" | "functionBlock"; + /** "configuration" covers CONFIGURATION VAR_GLOBAL ... AT. Those live in + * ast.configurations[].varBlocks rather than ast.globalVarBlocks, so they are + * gathered during validation (collectConfigurationLocatedVars) instead of + * during symbol building. */ + scopeType: "program" | "function" | "functionBlock" | "configuration"; scopeName: string; declaration: VarDeclaration; } @@ -555,8 +583,23 @@ export class SemanticAnalyzer { address: decl.address, }); - // Track located variables for validation - if (decl.address) { + // Track located variables for validation. + // + // Only VAR and VAR_GLOBAL may own an address (see + // LOCATABLE_BLOCK_TYPES). Report and do NOT record the declaration, + // so a located VAR_EXTERNAL cannot also collide with the global that + // legitimately claims the address. + if (decl.address && !LOCATABLE_BLOCK_TYPES.has(block.blockType)) { + this.addError( + `Variable '${name}' in ${block.blockType} cannot have a location ('AT ${decl.address}'). Only VAR and VAR_GLOBAL declarations may be located.` + + (block.blockType === "VAR_EXTERNAL" + ? ` A VAR_EXTERNAL references storage owned by a CONFIGURATION VAR_GLOBAL — declare the address on that VAR_GLOBAL and drop it here.` + : ` Move '${name}' to a VAR block, or to CONFIGURATION VAR_GLOBAL if other POUs need it.`), + decl.sourceSpan.startLine, + decl.sourceSpan.startCol, + decl.sourceSpan.file, + ); + } else if (decl.address) { const parsed = parseAddress(decl.address); if (parsed) { this.locatedVars.push({ @@ -604,7 +647,7 @@ export class SemanticAnalyzer { this.validateUndeclaredVariables(ast); // Validate located variables - this.validateLocatedVariables(); + this.validateLocatedVariables(ast); // Validate CONSTANT assignment restrictions this.validateConstantAssignments(ast); @@ -730,18 +773,104 @@ export class SemanticAnalyzer { } } + /** + * Collect located CONFIGURATION VAR_GLOBALs. + * + * These are NOT gathered by buildVarBlockSymbols: that runs per POU scope + * (program / function / functionBlock) over ast.programs et al, while + * configuration globals live in ast.configurations[].varBlocks. Without this + * they escaped every located-variable rule, so a POU-local `VAR ... AT %MX0.0` + * and a `VAR_GLOBAL ... AT %MX0.0` could both claim the same image slot — and + * they are serviced by different paths (the owning task vs. the dispatcher at + * the quiescent frame boundary), which makes the outcome nondeterministic. + * + * Globals sharing a name across configurations are one canonical global (codegen + * emits a single file-scope singleton, deduping by name), so dedupe here too — + * otherwise a project declaring the same global in two configurations would + * report a spurious duplicate-address error against itself. + */ + private collectConfigurationLocatedVars( + ast: CompilationUnit, + ): LocatedVarInfo[] { + const collected: LocatedVarInfo[] = []; + const seen = new Set(); + + for (const config of ast.configurations) { + for (const block of config.varBlocks) { + if (block.blockType !== "VAR_GLOBAL") continue; + for (const decl of block.declarations) { + if (!decl.address) continue; + for (const name of decl.names) { + const key = name.toUpperCase(); + if (seen.has(key)) continue; + seen.add(key); + + const parsed = parseAddress(decl.address); + if (!parsed) { + this.addError( + `Invalid address format: ${decl.address}`, + decl.sourceSpan.startLine, + decl.sourceSpan.startCol, + decl.sourceSpan.file, + ); + continue; + } + collected.push({ + name, + address: decl.address, + parsed, + typeName: decl.type.name, + scopeType: "configuration", + scopeName: config.name, + declaration: decl, + }); + } + } + } + } + return collected; + } + + /** + * How many times each PROGRAM type is instantiated across all configurations. + * Keyed by upper-cased program type name. + */ + private countProgramInstantiations( + ast: CompilationUnit, + ): Map { + const counts = new Map(); + for (const config of ast.configurations) { + for (const resource of config.resources) { + for (const instance of resource.programInstances) { + const key = instance.programType.toUpperCase(); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + } + } + return counts; + } + /** * Validate located variables for IEC 61131-3 compliance. * Checks: * - Located variables not allowed in function blocks - * - No duplicate addresses + * - Located variables not allowed in a PROGRAM instantiated more than once + * - No duplicate addresses (POU-local and configuration globals together) * - Type must be compatible with address size * - Bit index must be 0-7 for bit addresses */ - private validateLocatedVariables(): void { + private validateLocatedVariables(ast: CompilationUnit): void { const addressMap = new Map(); + const instanceCounts = this.countProgramInstantiations(ast); - for (const locVar of this.locatedVars) { + // Configuration globals participate in every rule below, above all in the + // duplicate-address check they were previously invisible to. + const allLocatedVars = [ + ...this.locatedVars, + ...this.collectConfigurationLocatedVars(ast), + ]; + + for (const locVar of allLocatedVars) { const decl = locVar.declaration; // Rule 1: Located variables not allowed in function blocks @@ -755,6 +884,32 @@ export class SemanticAnalyzer { continue; } + // Rule 1b: a fully specified address cannot live in a PROGRAM that is + // instantiated more than once. Same reasoning as Rule 1 for function + // blocks: a physical address belongs to exactly one point of hardware, so + // several instances of one POU type cannot each own it. IEC 61131-3 permits + // multiple program instances, and its answer for per-instance addressing is + // a partly specified location (`AT %I*`) resolved by VAR_CONFIG — which is + // not supported here, so the fully specified form must be rejected. + // + // Left unchecked this fails silently rather than loudly: codegen allocates + // one locatedVars[] slot per *declaration*, and each instance's constructor + // overwrites its pointer, so the last instance constructed wins and the + // other instances' copies of the variable are never serviced at all. + if (locVar.scopeType === "program") { + const instances = + instanceCounts.get(locVar.scopeName.toUpperCase()) ?? 0; + if (instances > 1) { + this.addError( + `Located variable '${locVar.name}' at ${locVar.address} not allowed in PROGRAM '${locVar.scopeName}': the program is instantiated ${instances} times, and a physical address cannot be shared by several instances. Declare the variable in CONFIGURATION VAR_GLOBAL and access it with VAR_EXTERNAL, or instantiate '${locVar.scopeName}' only once.`, + decl.sourceSpan.startLine, + decl.sourceSpan.startCol, + decl.sourceSpan.file, + ); + continue; + } + } + // Rule 2: Validate type compatibility with address size const compatibleTypes = getCompatibleTypes(locVar.parsed.size); if (!compatibleTypes.includes(locVar.typeName.toUpperCase())) { diff --git a/tests/fixtures/smart-traffic-light/src/Programs/IntersectionController.st b/tests/fixtures/smart-traffic-light/src/Programs/IntersectionController.st index 33492a29..7aad9b11 100644 --- a/tests/fixtures/smart-traffic-light/src/Programs/IntersectionController.st +++ b/tests/fixtures/smart-traffic-light/src/Programs/IntersectionController.st @@ -4,16 +4,18 @@ traffic lights, pedestrian signals, and vehicle detection. *) PROGRAM IntersectionController - VAR_INPUT + (* Located I/O belongs in VAR, not in the interface sections. Only VAR and + VAR_GLOBAL may carry an "AT" location: VAR_INPUT / VAR_OUTPUT describe a + call contract, and a located interface declaration would leave the runtime + with two owners for one image slot. *) + VAR (* Sensor inputs — mapped to physical I/O *) northSensor AT %IX0.0 : BOOL; southSensor AT %IX0.1 : BOOL; eastSensor AT %IX0.2 : BOOL; westSensor AT %IX0.3 : BOOL; pedestrianButton AT %IX0.4 : BOOL; - END_VAR - VAR_OUTPUT (* Signal outputs — mapped to physical I/O *) northRed AT %QX0.0 : BOOL; northYellow AT %QX0.1 : BOOL; diff --git a/tests/integration/compile.test.ts b/tests/integration/compile.test.ts index e0c55cfc..8ef810de 100644 --- a/tests/integration/compile.test.ts +++ b/tests/integration/compile.test.ts @@ -469,9 +469,12 @@ describe('Error Handling Tests', () => { // gated (fail-loud) until locked field/element/call codegen lands. describe('Shared globals (mutex model)', () => { it('compiles a scalar located shared global end-to-end', () => { + // The address lives on the CONFIGURATION VAR_GLOBAL, which owns the + // storage; the VAR_EXTERNAL only names it. Repeating `AT` on the external + // is rejected (see 'rejects a located VAR_EXTERNAL' below). const source = ` PROGRAM Main - VAR_EXTERNAL run AT %QX0.0 : BOOL; END_VAR + VAR_EXTERNAL run : BOOL; END_VAR VAR seed : BOOL := TRUE; END_VAR run := seed; END_PROGRAM diff --git a/tests/semantic/located-variables.test.ts b/tests/semantic/located-variables.test.ts index cf1b6354..eec81e7b 100644 --- a/tests/semantic/located-variables.test.ts +++ b/tests/semantic/located-variables.test.ts @@ -226,6 +226,288 @@ describe('Phase 2.3 - Located Variables', () => { }); }); + // Only VAR and VAR_GLOBAL may own an address; interface sections describe a + // call contract, not hardware. Enforced here to match the editor + // (DISALLOWED_LOCATION_CLASSES, GitHub issue #904) — and because a located + // interface declaration hands the runtime two owners for one image slot, a + // hazard strucpp cannot see from the generated code alone. + describe('Semantic: Only VAR and VAR_GLOBAL May Be Located', () => { + // VAR_EXTERNAL is covered separately below: without a matching VAR_GLOBAL a + // pre-existing rule reports the missing global first. + const cases: Array<[string, string]> = [ + ['VAR_INPUT', 'VAR_INPUT inp AT %IX0.0 : BOOL; END_VAR'], + ['VAR_OUTPUT', 'VAR_OUTPUT outp AT %QX0.1 : BOOL; END_VAR'], + ['VAR_IN_OUT', 'VAR_IN_OUT io AT %QX0.2 : BOOL; END_VAR'], + ]; + + for (const [label, block] of cases) { + it(`rejects a located ${label}`, () => { + const result = compile(` + PROGRAM Main + ${block} + VAR local : BOOL; END_VAR + local := local; + END_PROGRAM + `); + expect(result.success).toBe(false); + expect( + result.errors.some(e => + e.message.includes('Only VAR and VAR_GLOBAL declarations may be located'), + ), + ).toBe(true); + }); + } + + it('allows a located plain VAR in a PROGRAM', () => { + const result = compile(` + PROGRAM Main + VAR + sensor AT %IX0.0 : BOOL; + lamp AT %QX0.0 : BOOL; + END_VAR + lamp := sensor; + END_PROGRAM + `); + expect(result.success).toBe(true); + }); + + it('points a located VAR_EXTERNAL at the right fix', () => { + const result = compile(` + PROGRAM Main + VAR_EXTERNAL run AT %QX0.0 : BOOL; END_VAR + run := run; + END_PROGRAM + CONFIGURATION Config0 + VAR_GLOBAL run AT %QX0.0 : BOOL; END_VAR + RESOURCE Res0 ON PLC + TASK t(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM p WITH t : Main; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(false); + // Must NOT be reported as a duplicate address against the global it names. + expect(result.errors.some(e => e.message.includes('Duplicate address'))).toBe(false); + expect( + result.errors.some(e => + e.message.includes('references storage owned by a CONFIGURATION VAR_GLOBAL'), + ), + ).toBe(true); + }); + + it('allows the corrected form: address on the global, plain VAR_EXTERNAL', () => { + const result = compile(` + PROGRAM Main + VAR_EXTERNAL run : BOOL; END_VAR + run := run; + END_PROGRAM + CONFIGURATION Config0 + VAR_GLOBAL run AT %QX0.0 : BOOL; END_VAR + RESOURCE Res0 ON PLC + TASK t(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM p WITH t : Main; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(true); + }); + }); + + // Configuration VAR_GLOBALs live in ast.configurations[].varBlocks, not + // ast.globalVarBlocks, so they used to bypass every located-variable rule. + describe('Semantic: Configuration VAR_GLOBAL Participates In Address Checks', () => { + it('errors when a POU-local var collides with a located global', () => { + // Worst case for the runtime: the POU-local entry is serviced by the owning + // task while the global is serviced by the dispatcher at the quiescent + // frame boundary, so two paths write one image bit. + const result = compile(` + PROGRAM Main + VAR s AT %MX0.0 : BOOL; END_VAR + s := s; + END_PROGRAM + CONFIGURATION Config0 + VAR_GLOBAL g AT %MX0.0 : BOOL; END_VAR + RESOURCE Res0 ON PLC + TASK t(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM p WITH t : Main; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(false); + expect(result.errors.some(e => e.message.includes('Duplicate address %MX0.0'))).toBe(true); + }); + + it('errors on two located globals at the same address', () => { + const result = compile(` + PROGRAM Main + VAR x : BOOL; END_VAR + x := x; + END_PROGRAM + CONFIGURATION Config0 + VAR_GLOBAL + g1 AT %MX0.0 : BOOL; + g2 AT %MX0.0 : BOOL; + END_VAR + RESOURCE Res0 ON PLC + TASK t(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM p WITH t : Main; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(false); + expect(result.errors.some(e => e.message.includes('Duplicate address %MX0.0'))).toBe(true); + }); + + it('validates type compatibility on located globals too', () => { + const result = compile(` + PROGRAM Main + VAR x : BOOL; END_VAR + x := x; + END_PROGRAM + CONFIGURATION Config0 + VAR_GLOBAL bad AT %MX0.0 : INT; END_VAR + RESOURCE Res0 ON PLC + TASK t(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM p WITH t : Main; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(false); + expect(result.errors.some(e => e.message.includes('not compatible with address size'))).toBe( + true, + ); + }); + + it('accepts the same global declared in two configurations (one canonical global)', () => { + // Codegen dedupes file-scope globals by name, so this must not be reported + // as a duplicate address against itself. + const result = compile(` + PROGRAM Main + VAR_EXTERNAL g : BOOL; END_VAR + g := g; + END_PROGRAM + CONFIGURATION Config0 + VAR_GLOBAL g AT %MX0.0 : BOOL; END_VAR + RESOURCE Res0 ON PLC + TASK t(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM p WITH t : Main; + END_RESOURCE + END_CONFIGURATION + CONFIGURATION Config1 + VAR_GLOBAL g AT %MX0.0 : BOOL; END_VAR + RESOURCE Res1 ON PLC + TASK t2(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM p2 WITH t2 : Main; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.errors.some(e => e.message.includes('Duplicate address'))).toBe(false); + }); + }); + + // A physical address belongs to one point of hardware, so a POU type that is + // instantiated more than once cannot own one. Same reasoning as the FUNCTION_BLOCK + // restriction. Left unchecked it fails silently: codegen allocates one + // locatedVars[] slot per declaration and each instance's constructor overwrites + // its pointer, so only the last instance constructed is ever serviced. + describe('Semantic: Located Variables In Multiply-Instantiated Programs', () => { + const worker = ` + PROGRAM worker + VAR sensor AT %IX3.0 : BOOL; seen : BOOL; END_VAR + seen := sensor; + END_PROGRAM + `; + + it('errors when the program is instantiated twice in different tasks', () => { + const result = compile(` + ${worker} + CONFIGURATION Config0 + RESOURCE Res0 ON PLC + TASK t1(INTERVAL := T#20ms, PRIORITY := 1); + TASK t2(INTERVAL := T#50ms, PRIORITY := 2); + PROGRAM i1 WITH t1 : worker; + PROGRAM i2 WITH t2 : worker; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(false); + expect( + result.errors.some(e => e.message.includes('instantiated 2 times')), + ).toBe(true); + }); + + it('errors when the program is instantiated twice in the SAME task', () => { + // Task count is irrelevant — the trigger is instance count. + const result = compile(` + ${worker} + CONFIGURATION Config0 + RESOURCE Res0 ON PLC + TASK t1(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM i1 WITH t1 : worker; + PROGRAM i2 WITH t1 : worker; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(false); + expect( + result.errors.some(e => e.message.includes('instantiated 2 times')), + ).toBe(true); + }); + + it('allows a single instance', () => { + const result = compile(` + ${worker} + CONFIGURATION Config0 + RESOURCE Res0 ON PLC + TASK t1(INTERVAL := T#20ms, PRIORITY := 1); + PROGRAM i1 WITH t1 : worker; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(true); + }); + + it('allows multiple instances when the program declares no located variable', () => { + const result = compile(` + PROGRAM plain + VAR c : INT; END_VAR + c := c + 1; + END_PROGRAM + CONFIGURATION Config0 + RESOURCE Res0 ON PLC + TASK t1(INTERVAL := T#20ms, PRIORITY := 1); + TASK t2(INTERVAL := T#50ms, PRIORITY := 2); + PROGRAM i1 WITH t1 : plain; + PROGRAM i2 WITH t2 : plain; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(true); + }); + + it('allows multiple instances sharing a located global via VAR_EXTERNAL', () => { + // The sanctioned way for several POUs to reach one physical point: the + // configuration owns the address, instances only reference it. + const result = compile(` + PROGRAM reader + VAR_EXTERNAL shared : BOOL; END_VAR + VAR seen : BOOL; END_VAR + seen := shared; + END_PROGRAM + CONFIGURATION Config0 + VAR_GLOBAL shared AT %MX0.0 : BOOL; END_VAR + RESOURCE Res0 ON PLC + TASK t1(INTERVAL := T#20ms, PRIORITY := 1); + TASK t2(INTERVAL := T#50ms, PRIORITY := 2); + PROGRAM i1 WITH t1 : reader; + PROGRAM i2 WITH t2 : reader; + END_RESOURCE + END_CONFIGURATION + `); + expect(result.success).toBe(true); + }); + }); + describe('Semantic: Type Size Compatibility', () => { it('should accept BOOL for bit address', () => { const source = `