Skip to content

refactor(shader): split parser/compiler/analyzer + structured diagnostics (RFC #3017)#3054

Draft
zhuxudong wants to merge 135 commits into
galacean:dev/2.0from
zhuxudong:refactor/shader-analyzer
Draft

refactor(shader): split parser/compiler/analyzer + structured diagnostics (RFC #3017)#3054
zhuxudong wants to merge 135 commits into
galacean:dev/2.0from
zhuxudong:refactor/shader-analyzer

Conversation

@zhuxudong

@zhuxudong zhuxudong commented Jun 26, 2026

Copy link
Copy Markdown
Member

设计文档:RFC #3017(完整诊断规则目录、判据、实施步骤见该 issue)

背景与目标

dev/2.0 的 shader 诊断与运行时编译耦合在一包,靠 #if _VERBOSE 门控:release 产物零诊断、错误无行列定位、同一错误多次上报、外部无法消费。

本 PR 把 shader 体系拆为三包并引入结构化诊断:

  • 解耦:诊断与 codegen 物理分离,依赖结构上不可能 drift;
  • 结构化:可定位、release 可见、每错一遍、IDE / CI / LLM 可直接消费。

架构

flowchart LR
    SRC["ShaderLab 源码"] --> P["📦 shader-parser<br/>中性 typed-AST + 语义线索<br/>(唯一来源)"]
    P --> C["⚙️ shader-compiler<br/>codegen → GLSL ES"]
    P --> A["🔍 shader-analyzer<br/>ShaderValidator 校验 + 收集线索"]
    C --> OUT["IShaderProgramSource"]
    A --> DIAG["Diagnostic[]"]
    classDef base fill:#fff9c4,stroke:#f57f17
    classDef comp fill:#d4edda,stroke:#28a745
    classDef ana fill:#e3f2fd,stroke:#1976d2
    class P base
    class C,OUT comp
    class A,DIAG ana
Loading

shader-compilershader-analyzer 互不依赖,是 parser 之上两个平等消费者。类型/控制流校验抽成独立的 ShaderValidator pass(走 typed-AST),类型语义收口到 TypeSystem;与解析序耦合的校验(重载 / 声明序 / const 求值)+ 管线 IO + RenderState 仍在 parser 内联打标。整体对齐 Naga「中性源 + 独立 Validator + 多 backend」的分离方向(建模型 / 校验 / 发射);模型是带注解的 typed-AST,非完全 lower 的 IR。

核心机制:校验独立于 codegen

dev/2.0 本 PR
判定 散落 reportError + #if _VERBOSE 类型/控制流校验由 ShaderValidator 走 typed-AST 现算;解析序耦合项 parser 内联打标(error-as-clue)
analyzer ShaderValidator 走 typed-AST 做类型/控制流校验 + 收集 parser 线索 → Diagnostic[]
codegen 与诊断耦合 纯生成、零诊断;走同一棵 typed-AST

对外 API

入口 形态 场景
analyzer.analyze(src) 返回结构化 Diagnostic[] 编辑器 / CI / LLM
WebGLEngine.create({ shaderCompiler, shaderAnalyzer }) 注入即诊断 → Logger dev / 运行时校验
formatDiagnostic(d) 渲染带行号源码 + caret 的文本块 运行时 Logger 与编辑器/CI 同款输出
interface Diagnostic {
  severity: DiagnosticSeverity;   // error | warning(枚举)
  code: DiagnosticType;           // 语义码(glslang 风格,非数字码)
  message: string;                // 'token' : 说明
  range: { start; end };          // 行 / 列 / offset
  relatedSource?: string;         // 出错 pass 源码,供上下文呈现
}

诊断能力

43 个 DiagnosticType,逐条对 Naga valid/ 源码核过粒度(过度拆分已合并、ShaderLab/GLSL 专属项标注依据);完整目录 + 正/误示例见 RFC #3017 §3。按实体分类:

覆盖
结构 入口绑定、入口指向、RenderState 合法性
类型 / 表达式 swizzle、struct 成员、构造函数、下标、除零、移位、运算操作数类型
符号 / 作用域 使用前声明、重定义、函数解析、实参匹配
函数 / 控制流 void/return、if 条件、break/continue、禁递归
管线 IO 入口签名、IO 可共享类型、struct 角色、整型 varying flat、vertex 输出位置
常量 / 资源 数组大小常量、const 初始化、采样目标

优势

  • 物理解耦:依赖结构强制 single source of truth;release 可见诊断;取消 verbose 变体(8 → 4 产物)。
  • 平台无关:类型/控制流诊断建立在 parser 的中性 typed 线索上,与具体后端无关;平台分叉构造(如 array-of-array)归 backend 生成时报。
  • 零运行时成本:不注入 analyzer 即纯编译;shader codegen 输出保持不变。
  • 可消费:结构化 Diagnostic —— 编辑器画波浪线、CI 出 file:line:col [Code] msg、LLM 拿去自动修复。

测试

  • 每条诊断规则配 ABTest:正确版(应通过、零该类型诊断)+ 错误版(精确命中 DiagnosticType + 位置)。
  • codegen 回归保护:基于 dev/2.0,IShaderProgramSource 字节对等(precompile 守护)。
  • 覆盖率门:每个 DiagnosticType 必有触发用例,缺测即 fail。
  • 本地与 CI:npm run buildb:module + b:types,tsc 全量类型检查)后 npm run test

zhuxudong added 30 commits June 2, 2026 11:07
- move createPosition/createRange + their pools to ShaderCompilerUtils
- move pass-text error context to ShaderCompilerUtils.processingPassText
- add ICodeGenVisitor interface so AST no longer imports concrete CodeGenVisitor
- parser/lexer/codegen now depend on ShaderCompilerUtils, not ShaderCompiler entry

prep for extracting shared shader-parser package (c3); no behavior change, 197 tests green
- copy ClearableObjectPool/IPoolElement into local common/ObjectPool
- add local no-op Logger (engine-core Logger is also disabled by default)
- copy render-state enums into common/enums/RenderStateEnums (values mirror engine-core)
- parser/lexer/lalr/sourceParser now engine-core-free; engine-math (Color) kept as foundation dep

deviates from RFC: Color kept as engine-math dep, render-state enums copied; 197 tests green
…mpiler

- move lexer/preprocessor/parser/lalr/AST/sourceParser + utils into @galacean/engine-shader-parser
- shader-compiler depends on it; cross-package imports go through the package barrel
- shader-parser ships one always-full build (jscc _VERBOSE=true), external to shader-compiler
- shader-parser drops stripInternal so compiler/analyzer can use internal parser APIs

pure relocation; 197 shader-compiler tests green
…gnostics

- new @galacean/engine-shader-analyzer drives the parse + collects diagnostics, skips codegen
- restores diagnostics the runtime compiler discards (parity verified vs verbose compiler)
- harvest approach: checks stay in shader-parser (single source), no visitor duplication
- Phase 1 returns GSError verbatim; structured API + new checks are Phase 2

harvest deviates from RFC's DiagnosticVisitor plan; 199 tests green
- analyzer now runs codegen too, capturing codegen-level diagnostics (struct/MRT/gl_FragData)
- ungate codegen error collection so the single release build always collects them
- remove ShaderCompiler._logErrors + calls: the compiler compiles, never reports
- delete the verbose build variant (/verbose export, rollup push, stub dir)
- shader-compiler drops stripInternal + exports GLES visitors so analyzer can drive codegen

completes Phase 1: diagnostics live in the analyzer; 200 tests green
- drop unused ObjectPool.garbageCollection (pools reuse via clear(), never GC)
- remove dead verboseMode branches from root rollup (no verbose build remains)
- collapse duplicate glslValidate calls left by the verbose→release test switch
- drop an obsolete warning-spy guard (the warning no longer exists; macro asserts cover it)
- tighten comments: drop task-context and a claim of a non-existent sync test
- remove unused abstract ObjectPool base class (only ClearableObjectPool
  extends it; inline the two fields)
- replace indirect ReturnType<typeof ShaderSourceParser.parse> with IShaderSource
- Diagnostic interface (severity, code, range, message, source, relatedSource)
- DiagnosticCode registry: C0 (parser/codegen), A1 (ShaderLab), B1/B2 (RenderState)
- gseErrorToDiagnostic converts GSError to structured Diagnostic
- ShaderAnalyzer.analyze() returns AnalysisResult.diagnostics: Diagnostic[]
- heuristic code mapping from GSErrorName + message content
- tests verify structured output for all 3 diagnostic sources
- reportWarning routed to Logger.warn, a noop since Phase 1 decoupled Logger
- the "declared before used" warning was silently dropped as a result
- now push CompilationWarn to errors[] (gated by _VERBOSE, like reportError)
- analyzer surfaces it as a C0-07 warning diagnostic; drop unused Logger import
- a failed function lookup is signature-keyed, conflating unknown names and wrong-arg calls
- both surfaced as one opaque "No overload function type found" message
- re-probe by name alone (+ builtin registry) to split the two cases
- unknown names now report a distinct "Undefined function" (C0-09); wrong-args keeps C0-06
- shader-parser always builds _VERBOSE=true, so its 88 #if _VERBOSE blocks were dead scaffolding
- the guarded code (diagnostics, line/column tracking) already shipped in every build
- strip all markers + drop the 2 dead #else console.error fallbacks
- dist and behavior identical to before; 202 shader tests stay green
- rollup _VERBOSE jscc is now a no-op (zero #if _VERBOSE left repo-wide) — remove it + the import
- VisitorContext location: any -> BaseToken["location"]; IRenderState drops the pointless | any
- BaseLexer throwError msgs: any[] -> unknown[] (only ever join()'d)
- map the "referenced X not found" codegen error to a dedicated C0-22 instead of the C0-08 fallback
- SymbolTable.insert silently overwrote a same-scope duplicate via a now-noop Logger.warn
- insert() now returns whether it replaced an equal symbol; decl sites surface it as a C0-10 warning
- macro-branch siblings stay exempt (insert skips isInMacroBranch entries) — covered by a test
- applies to local (SingleDeclaration/InitDeclaratorList) and global (VariableDeclaration) vars
- add PostfixExpression.semanticAnalyze: a `.field` on a known vector is validated as a swizzle
- catches out-of-range components (.z on vec2), mixed sets (.xr), bad chars, length > 4
- only fires when the base type is a concrete vecN — struct members and unresolved bases skip
- ParserUtils.swizzleError holds the rule; first C1 (GLSL type) layer check
- AssignmentExpression flags `a = b` when b's type cannot convert to a's
- ParserUtils.isAssignable models GLSL ES3 implicit conversions (int->float, ivecN->vecN)
- so valid coercions (float = int) are NOT flagged; only definite conflicts surface
- fires only when both operand types are concrete; compound RHS / structs are skipped
- JumpStatement.semanticAnalyze checks `return expr` against the function's declared return type
- reuses ParserUtils.isAssignable, so implicit conversions (return int from a float fn) pass
- skips void returns (C0-04 covers those) and unresolved/compound expressions
- ShaderTargetParser singleton: a failed parse (syntax error) left _traceBackStack dirty
- the next parse was then corrupted: a valid shader got a spurious diagnostic
- this hits the runtime compiler too (ShaderCompiler/ShaderAnalyzer share the singleton)
- clear _traceBackStack each parse; reset SymbolTableStack._macroLevel in clear() too
- regression test: a broken analyze() must not corrupt the following valid one
- registerRule(rule) runs user rules after the built-in checks on every analyze()
- rules get source + parsed structure + positionAt(); report() namespaces the code as <name>/<code>
- a throwing rule surfaces a <name>/rule-error warning instead of crashing analysis
- analyze() restructured so rules run even when structure parsing fails (built-in path unchanged)
- src/shader-playground.ts: a live editor + diagnostics panel driven by ShaderAnalyzer.analyze()
- no engine init (analyzer is standalone); the sample shows C0-09/10 and C1-01/02/03
- also demos registerRule via a "demo/no-discard" custom rule
- wire engine-shader-analyzer into examples deps + vite optimizeDeps exclude
- upgrade shader-parser's local Logger to a real controllable one (enable/disable, off by default)
- keeps zero engine-core dependency (local copy mirroring engine-core's API)
- route runtime console.* through it: error prints -> Logger.error, version banner -> Logger.info
- the compiler version banner no longer prints on every import (silent unless logging enabled)
- remove dead debug dumpers printStatePool / _printStack (uncalled) and their console
- bundler CLI keeps console (build-time terminal output); shader-analyzer had no bare console
- local common/Logger.ts existed to avoid an engine-core dep, but core never imports shader pkgs
- core injects shader-compiler (no import), so there was never a cycle to avoid
- depend on engine-core and use its Logger; redirect 4 parser + 1 compiler imports, drop the copy
- logging now unifies with the engine's Logger; 1428 tests pass, compiledShaders byte-identical
- the diagnostic package now logs each diagnostic via the engine Logger, off by default
- severity-mapped: error->error, warning->warn, info->info, hint->debug
- add @galacean/engine-core dep; analyze() logs after collecting all diagnostics
- enable Logger to see every syntax/semantic problem in the console while analyzing
…cal copy

- common/ObjectPool.ts was a local copy of core's pool (made to avoid the now-cycle-free core dep)
- core's ClearableObjectPool/IPoolElement are behavior-identical (same get/clear logic)
- redirect the 6 import sites to @galacean/engine-core; drop the local copy + its re-export
- 213 shader tests pass, compiledShaders byte-identical to dev/2.0
…al copy

- common/enums/RenderStateEnums.ts was a hand-synced local copy of core's 8 render-state enums
- drop it; ShaderSourceParser imports them from @galacean/engine-core (merged into its core import)
- no re-export consumer; design types render state by number, so no enum-identity issue at boundary
- compiledShaders byte-identical (render-state serialization unchanged); 213 tests pass
- rollup.config.js: drop the dead jscc plugin (no #if _VERBOSE left) + its stale verbose comments
- also drop a dangling src/enums/README.md reference in that file's header
- convert.ts: drop the "Phase 2 ... DiagnosticVisitor" promise (no DiagnosticVisitor was built)
- Preprocessor/Lexer: fix comments referencing the removed verbose build / wrong package
- drop ShaderInstructionEncoder's hand-synced local copy of the directive enum

- core now exports ShaderPreprocessorDirective publicly (values unchanged Text=0..Undef=10)

- compiledShaders stay byte-identical to dev/2.0; tsc clean across the 3 shader packages
…e registry

- gSErrorNameToCode returns DiagnosticCode.* refs, dropping 34 duplicated raw "C0-xx" literals

- return type is DiagnosticCodeValue so tsc rejects any code absent from the registry

- remove the never-passed defaultSeverity param (all five callers use the default)
- add SemanticWalker that walks the built AST and derives diagnostics from node type clues

- PostfixExpression still produces the type clue; swizzle check (C1-01) leaves its semanticAnalyze

- establishes parser-produces-clues / analyzer-judges pattern; first step of diagnostics decoupling

- compiledShaders byte-identical; full suite 1429 pass
- IntegerConstantExpressionOperator.compute is now optional; absence = unknown-operator clue

- C0-02 judgment leaves parser semanticAnalyze for the walker

- compiledShaders byte-identical; full suite 1429 pass
- remove SemanticWalker; swizzle (C1-01) and operator (C0-02) judgments go back to parser

- analyzer-side instanceof was a hack; judgment belongs internalized in parser clue computation

- correct model: error-as-clue in parser + analyzer generic collection

- compiledShaders byte-identical; 1429 tests pass
zhuxudong added 30 commits July 8, 2026 17:56
The check only flagged direct sampler return; a comment claimed the
struct case was handled but the code never did. Now looks the struct up
in the shader symbol table and walks its members, recursively descending
into nested structs and using a visited set to break typedef/macro cycles.
Both leaf sampler and struct-containing-sampler return types now fire.
isConstExpr only recognised literals and bare const/macro identifiers.
Extended to:
- built-in function calls (constructors + sin/cos/pow/... via
  BuiltinFunction.isExist) whose args are themselves constant
- compound expressions (binary/unary/ternary) where every sub-expression
  is constant

Short-circuits on the first non-constant operand so `u_uniform + sin(0.5)`
still reports (uniform mixed in) while `sin(0.5)` and `1.0 + 2.0` don't.

Only invoked from the NonConstInitializer / NonConstArraySize diagnostics
in the parser semantic pass — codegen never reads this predicate so it
adds zero runtime cost to shader compilation.
GLSL ES §4.1.9: array size must be a positive integer. Real WebGL drivers
reject `float a[0];` with "array size must be greater than zero" but we
silently accepted it and only failed later at WebGL compile time.

New DiagnosticType.InvalidArraySize fires when the literal-folded size
evaluates to zero or a negative number. Non-literal / undefined sizes
still fall through to the const-expression check.
Direct self-call was already covered. Added an SCC post-pass over the
call graph built during the walk: every `FunctionCallGeneric` edge is
recorded regardless of whether it's a self-call, and after the walk we
iterate over the graph looking for cycles of length ≥ 2. Each cycle is
reported once, at the lexicographically-first participant's declaration.

The playground sample keeps only direct recursion because our grammar
doesn't accept function forward declarations — mutual recursion is
grammatically unreachable in ShaderLab today. The pass still fires when
a future grammar change unlocks it, or when a mutually-recursive shape
sneaks through via macros.
Direct dFdx / dFdy / fwidth inside the vertex entry was already flagged
via WalkContext.currentStage. A helper called from the vertex entry stays
`currentStage === null`, so `float helper(float x) { return dFdx(x); }`
was silently accepted even though the vertex path evaluates it.

Now record every derivative call site by enclosing function name during
the walk. After the walk, transitively reach from the vertex entry via
the call graph and report each derivative site inside a reachable helper.
Helpers on the fragment-only path stay silent.
Both diagnostics fired once per reference — a shader that touched
gl_FragColor / gl_FragData three times reported the same conflict three
times. The condition is per-shader (mixed use of legacy and MRT outputs),
not per reference.

Report GlFragColorWithMrt at the first gl_FragColor reference and stop;
report GlFragData once per shader via a dedup flag on ShaderValidator
(covers both the bare-identifier and gl_FragData[i] paths).
- gl_FragData[i] is a valid GLSL ES 1.00 fragment output and does not require MRT
- remove the analyzer's guard (ShaderValidator._checkGlFragDataReference),
  DiagnosticType.GlFragData, its DIAGNOSTIC_CATEGORY entry, and the covering
  tests / playground sample
- codegen keeps its own MRT normalization; nothing else observes the removed code
… warnings

- undeclared identifiers (UseBeforeDeclaration) and unknown function names
  (UndefinedFunction) may resolve to runtime macros or conditional #include
  bodies that the precompile phase cannot see — reporting them as errors caused
  false positives for built-in shaders (e.g. RENDERER_JOINTS_NUM, texture2DLodOffset)
- overload mismatch on a KNOWN function name (NoMatchingOverload) stays an error;
  the arg-type contract is only meaningful once the callee is resolved
- NonConstArraySize now only fires when the size identifier resolves to a known
  non-const var — an undeclared identifier is already covered by the
  UseBeforeDeclaration warning, avoiding a duplicate/misleading error
…onsistency suite

- for each curated GLSL-body case: run the DSL through ShaderAnalyzer, run the
  same body through ShaderCompiler._parseShaderPass, then feed the emitted GLSL
  to a real WebGL context and check the three views agree
- severity contract: error → driver must reject; warning → driver behavior is
  not asserted (may be rescued by a runtime macro / conditional #include);
  no diagnostic → driver must accept
- 10 curated cases covering clean baseline, five error-severity diagnostics
  (AssignTypeMismatch, ConstructorArgCount, InvalidReturnType, IndexOutOfBounds,
  NoMatchingOverload, NonBoolCondition, MisplacedControlFlow) and the two
  identifier-lookup warnings (UseBeforeDeclaration, UndefinedFunction)
- also enforces "codegen never gates on severity" — every case, including
  error-severity ones, must still produce GLSL for editor / IDE consumption
…ic cases

- add 26 cases spanning Type / Symbol / ControlFlow / PipelineIO categories:
  InvalidSwizzle, UndeclaredStructMember, ConstDivideByZero, ShiftOutOfRange,
  NonIntegerIndex, NonIndexableType, ExpectedSampler, InvalidUnaryOperand,
  InvalidBinaryOperands, ConstructorArgType, NonConstInitializer,
  NonConstArraySize, InvalidArraySize, NonFloatDerivativeArg, MissingReturn,
  Redefinition, RecursiveFunction, NonConstructibleReturnType,
  DerivativeInVertexShader, InvalidEntryReturnType, StructRoleConflict,
  GlFragColorWithMrt, NestedIOStruct, MissingVertexPosition,
  NonFlatIntegerVarying, EntryNotFound
- relax the codegen assertion: `_parseShaderPass` may return undefined for
  parse-fail / pipeline-setup failures, but only if the analyzer produced an
  error — silent drops are still failures
- mark "either" for cases the driver may still fold (constant division /
  shift overflow) or optimize away (unreferenced sampler-returning function)
- RenderState-category and DSL-level entry diagnostics (DuplicateEntryAssignment,
  MissingEntry) are covered by DiagnosticCoverage.test.ts; they never reach
  the codegen path this suite drives
Two documentation-level defects from the ABC review:

- ShaderAnalyzer.test.ts:41 title said "error diagnostic" but the assertion
  is `severity === "warning"`; rename to "warning diagnostic"
- DiagnosticDriverConsistency.test.ts had the two warning-severity cases
  (UseBeforeDeclaration, UndefinedFunction) marked `driverExpects: "either"`
  with a comment explicitly admitting "the driver rejects" — the suite was
  documenting its own contract hole. Flip both to `"reject"`, keep `"either"`
  reserved for spec-undefined cases (const div-by-zero, shift overflow), and
  update the file header to make clear that warning severity encodes *intent*
  ("a runtime macro may rescue at bind time"), not driver acceptance
… use

- `gl_FragData` is a `vec4[]` fragment-output array; the driver only accepts
  `gl_FragData[i]` (an indexed subscript). Bare use as an l-value, r-value,
  swizzle base, or function argument is invalid GLSL — writing
  `gl_FragData = vec4(0.0)` or `vec4 c = gl_FragData` compiles to nothing
  meaningful and the driver rejects it
- collect every `gl_FragData` reference at parse time into
  `shaderData.glFragDataReferences` (mirroring `gl_FragColorReferences`);
  ShaderValidator's PostfixExpression check records the base of each legal
  `gl_FragData[i]` shape in `_indexedGlFragDataStarts`
- after the walk, `_reportBareGlFragData` subtracts the indexed set from the
  collected refs and reports the first residue as `BareGlFragData` (once per
  shader; the driver only needs one signal to reject)
- DiagnosticCoverage picks up the case; existing "gl_FragData[i] is legal"
  smoke test continues to pass (validates the strike-through path)
…codegen console

Two shader-diagnostics UX changes bundled by scope (both align the pipeline
with the analyzer-runtime decoupling stance):

- reword UseBeforeDeclaration and UndefinedFunction warnings from raw driver
  language ("undeclared identifier" / "Undefined function:") to a directive
  aimed at the shader author: "ensure it is provided at runtime (macro /
  #include)". Rationale is in-line: the analyzer is decoupled from the engine
  runtime by design and can't see macros that the material system feeds in at
  bind time, so it hands the responsibility back to the author instead of
  hard-failing at precompile
- drop `console.warn` from `GLESVisitor._softMissEntry`. The analyzer's
  `EntryNotFound` diagnostic already routes through the engine Logger for the
  user-facing signal; codegen is not the layer that surfaces UX, so precompile
  of built-in shaders (PBR, Blinn, etc.) never spams the browser console. The
  dedup Set and its `reset` call go with it — dead once the warn is gone
- `#include` directives are expanded by `Preprocessor.parse` before the AST is
  built, so the warning suggesting "provided at runtime (macro / #include)"
  was misleading — the only remaining "provided later" channel is a runtime
  macro from the material system
- reword UseBeforeDeclaration and UndefinedFunction to "ensure it is provided
  at runtime as a macro" and tighten the surrounding rationale comment
…mbol

- `VariableDeclaration.semanticAnalyze` (global scope path used by ShaderTarget-
  Parser) constructed VarSymbol without forwarding the `const` qualifier from
  `FullySpecifiedType.isConst`, so global `const float C = 1.0;` ended up with
  `VarSymbol.isConst = false`
- effect: any downstream check that treats `isConst` as authoritative (e.g. the
  new InvalidAssignmentTarget rule against writing to const-qualified variables)
  silently missed globals; SingleDeclaration (local scope) was already correct,
  so the gap only affected top-level `const`
- pass `type.isConst` as the fifth VarSymbol arg, mirroring the local path
GLSL ES §5.8: "the left operand of the assignment operator must be an l-value."
The parser only checked type compatibility on assignment (AssignTypeMismatch);
shapes that could never be written to went through silently — including the
`#define A 1; A = 2;` case that motivated this addition.

- new `DiagnosticType.InvalidAssignmentTarget` in the Type category
- `ShaderValidator._checkAssignmentTarget` runs on every AssignmentExpression
  and descends its LHS via `_nonAssignableReason`, mirroring the grammar's
  operator-precedence chain (single-child wrappers pass through, r-value-only
  shapes terminate with a specific reason)
- covered non-l-values:
    * macro reference (`MacroCallSymbol` / `MacroCallFunction`)
    * numeric / boolean literal
    * function-call result
    * `const`-qualified variable (lookup via `shaderData.symbolTable`)
    * compound arithmetic / logical / relational expressions
    * unary-operator result
    * ternary expression result
- DiagnosticDriverConsistency picks up five real-WebGL cases (macro, literal,
  function call, const, compound); DiagnosticCoverage adds the macro trigger
- follows naga's "one error kind, message describes the cause" convention
  rather than splitting per LHS shape
…s and assignments

Real WebGL 1 and WebGL 2 drivers reject the following with "cannot convert from
'const int' to 'mediump float'":

    float b = 1;      // initializer
    float b; b = 1;   // assignment
    float f = 1.0;
    float g = f + 1;  // binary op

The analyzer previously accepted all of these. The gap had two root causes:

- `TypeSystem.isAssignable` allowed the naga-style scalar promotion set (int →
  uint/float, uint → float, ivecN → uvecN/vecN, uvecN → vecN). GLSL ES §4 states
  the language has **no implicit conversions between types**; §5.8 (assignment)
  and §5.9 (binary operators) require operand types to match; §5.4.1 lists
  explicit scalar constructors as the only conversion mechanism. naga's
  `implicit_conversion` violates the spec — the driver is the source of truth,
  so tighten to `target === source`
- `SingleDeclaration` and `VariableDeclaration` never checked the initializer's
  type against the declared type — so `float b = 1;` slipped through. Both now
  emit `AssignTypeMismatch` on mismatch (array initializers skipped, they need
  component-level checking)

The two pre-existing "does not flag a valid implicit conversion" tests in
ShaderAnalyzer.test.ts were written from the wrong premise — flipped to assert
the diagnostic **must** fire, with in-comment rationale citing the spec.

DiagnosticDriverConsistency picks up three real-WebGL cases:
  * `float b = 1;` (initializer)
  * `int a = 1.0;` (initializer, other direction)
  * `float b; b = 1;` (assignment)

Built-in shader precompile stays clean — no engine shader relied on the removed
scalar promotion.
Every gap here was surfaced by porting a Khronos/glslang negative-test file
(`Test/300operations.frag`, `Test/invalidSwizzle.vert`, `Test/300scope.vert`) to
Galacean shaderlab DSL and running through a real WebGL driver. Each was a case
the analyzer silently accepted while the driver rejected — the exact class of
bug the ABC roleplay design misses (it only pressure-tests declared diagnostics,
never audits the diagnostic-code list against spec / corpora).

Fixes (9 gaps, 5 rule additions, 1 new diagnostic code):

- **G3 + G4 · InvalidSwizzle receiver-type check** (`ShaderValidator._checkPostfix`)
  `s.rr` (sampler receiver) and `f().xx` (void return) now report
  `Field selection ... requires a structure, vector, or scalar receiver`.
  §5.5 — receiver must be scalar/vector/struct; the driver rejects with
  "field selection requires structure, vector, or interface block on left
  hand side."
- **G2 · InvalidAssignmentTarget extended to uniform / sampler / `++`/`--`**
  `VarSymbol` gains `isUniform` (global var without an initializer, non-const —
  Galacean's implicit uniform). `_nonAssignableReason` now flags uniforms and
  samplers as non-l-values; `_checkPostfix` and `_checkUnaryOperand` route
  `++`/`--` through the same reason check. `u_i++` on a uniform is rejected by
  the driver as "l-value required (can't modify a uniform)".
- **G1 · Modulo (`%`) requires integer operands** (`_checkModuloOperandsInteger`)
  §5.9 says `%` is defined only on signed/unsigned integer scalar or vector.
  Floats slipped past `_checkArithmeticOperands` because they're a valid
  *arithmetic* type; the driver rejects with "wrong operand types."
- **G5 + G6 · Logical `&&`/`||`/`^^` require scalar bool operands**
  (`_checkScalarBoolBinaryOperands`) — §5.9 ES3 restricts these to `bool` only
  (not `bvecN`, unlike desktop GL). Covers `int && int` and `bool && bvec3`.
- **Shift + bitwise operators require integer operands**
  (`_checkIntegerBinaryOperands`) — §5.9. Sibling of the modulo check.
- **G7 + G8 · Arithmetic family mismatch** (`_checkArithmeticFamilyMatch`)
  `int + float`, `ivec3 + uvec3`, `uint + float` are all rejected because
  GLSL ES §4 has no implicit conversions between families. Only fires on
  direct-typed operands (respects Phase-2 constraint disabling compound-
  expression inference).
- **G9 · new `LocalFunctionPrototype` diagnostic** (`_checkLocalFunctionPrototype`)
  §6 restricts function prototypes to global scope. The grammar accepts
  `int g();` inside a function body; without this check the parser cascades
  into a misleading `EntryNotFound`. New Symbol-category diagnostic replaces
  the cascade with a targeted error.

Fixed a related parser gap the same round exposed:
`VariableDeclaration` (global scope) never propagated `type.isConst` into
`VarSymbol.isConst` — a global `const float C` was `isConst: false`. Now that
InvalidAssignmentTarget also relies on `isUniform`, the constructor takes both
flags derived from `FullySpecifiedType` and `children.length`.

Verification:
- 9 new cases in DiagnosticDriverConsistency drive each gap through the real
  WebGL driver — every one shows `driverExpects: "reject"` with the actual
  driver log cited in the case comment.
- New `LocalFunctionPrototype` picked up by DiagnosticCoverage.
- Built-in shader precompile still clean (no engine shader relied on the
  removed lenient behavior).
Verified against real WebGL: FXAA3_11.glsl uses `#define lumaN luma4B.z; ...
lumaN = lumaW;`, a legal swizzle l-value that both WebGL 1 and WebGL 2
drivers accept. The analyzer was categorically rejecting every macro-as-LHS
as `Cannot assign to a macro`, producing 2 false-positive errors on the
shipping FinalAntiAliasing post-processing shader.

A macro's l-value-ness depends on its EXPANSION, not on the fact that it's
a macro. Rejecting all macro-LHS is over-eager — the runtime driver already
catches genuine `#define K 3; K = 5;` after preprocess. So skip the macro
branch in `_nonAssignableReason`; keep the const / uniform / sampler /
literal / function-call / compound reasons intact.

Also add `BuiltinShaderSmoke.test.ts` — every shipping built-in shader is
now walked through `ShaderAnalyzer.analyze()` and asserted to fire no
`InvalidAssignmentTarget` / `InvalidSwizzle` / `InvalidBinaryOperands` /
`BareGlFragData` / `LocalFunctionPrototype` (the diagnostic codes this PR
sequence introduces or tightens). Prior verification only ran `precompile`
(codegen), which is why the FXAA regression shipped unnoticed.

Bug traced to `dfba45b5d` (the InvalidAssignmentTarget introduction, one
commit before this fix). Fix belongs here because the surrounding PR
extends the same `_nonAssignableReason` code path.
…y reason, IO defensive gate

- **InvalidVoidVariable** (§4.1.1) — `void x;` variable / parameter declarations now
  fire a specific error. Both `SingleDeclaration` (function-local) and
  `VariableDeclaration` (global) paths gate on `FullySpecifiedType.type === Keyword.VOID`.
  Driver rejects with `illegal use of type 'void'`; author gets a targeted message
  instead of a downstream cascade.
- **`_nonAssignableReason` sampler check order** — moved sampler-type check ahead of
  the isUniform branch. A sampler in ES is always uniform (§4.1.7), so both branches
  would fire; "a sampler" is a more actionable reason than the generic uniform text.
  Was previously dead code because the uniform check consumed the sampler case first.
- **`_deriveStructVarMap` defensive gate** — the `symbolTable.forEach` walk now gates
  on `sym.isGlobalVariable`. Currently unreachable because `popScope` always fires
  before validation, but if error recovery ever leaks a local into the top scope
  (e.g. from a partially parsed function), the map would misclassify it as a
  varying / attribute source. `VarSymbol.isGlobalVariable` is set at construction
  and is the correct axis.
…de `#if/#else`

Root cause: the analyzer walks every arm of `#if/#ifdef/#else` regions
simultaneously (there is no attempt at branch-selective analysis), so an
assignment `T x = 1.0;` where one arm defines `#define T vec3` and the other
`#define T float` produces a spurious `AssignTypeMismatch` in whichever arm
picks the wrong side of the cross-arm view. Same class of false positive drove
40+ `MissingReturn`s, plus `NonIndexableType` / `IndexOutOfBounds` /
`ConstructorArgCount` misfires on shipping shaders. `analyze()` over the
built-in corpus reported ~76 error-severity diagnostics none of which reflect
real bugs in any specific macro combination.

Fix: preserve the diagnostic (author still gets a signal), but downgrade its
severity to warning when the report site is inside a non-empty macro branch.
Empty-branch (top-level) diagnostics stay errors — those are unconditional
and reliable.

Wiring:
- `TreeNode._branch: BranchSignature` — inherited from the first child that
  carries a branch signature. Filled in `set()` at parse time, zero analyzer
  input required. Empty means unconditional.
- Parse-time reports (`SemanticAnalyzer.reportError`) consult
  `symbolTableStack.isInMacroBranch` (the live parser state) and route to
  `reportWarning` when active. `SyntaxError` bypasses this — it goes through
  `ShaderTargetParser` directly and stays a hard error.
- Post-parse reports (`ShaderValidator._push`) track `_macroBranchDepth`
  during `_walk`; every AST node with `_branch.length > 0` increments the
  depth for its subtree, and `_push` chooses `CompilationWarn` vs
  `CompilationError` based on the depth at report time.

Effect on `BuiltinShaderSmoke`:
- Before: 22 shipping shaders, 76 error-severity diagnostics, ~30 warnings
- After : 22 shipping shaders, 1 error-severity diagnostic (SkyProcedural
  vec4 vs float @l398, real unconditional issue — not this PR's concern),
  ~450 warnings preserving the signal
- Zero shader breaks the smoke test; the PR-introduced-codes gate stays green
…nt types across `#if/#else`

Per shader-analyzer scoping rule: within a scope, each name has exactly one
type — macros don't fork the type system. When a variable is declared with
different data types across mutually-exclusive `#if/#else` branches, whichever
type the analyzer picks for downstream lookups produces spurious
`AssignTypeMismatch` / `NonIndexableType` cascades. Report the real root cause
directly, listing each conflicting site (branch + type + location) so the fix
is unambiguous.

Implementation:
- New `DiagnosticType.MacroBranchConflict` (Symbol category)
- `ShaderValidator._reportMacroBranchConflicts` runs post-walk, groups
  `VarSymbol`s by name, and reports when at least one entry lives in a macro
  branch AND multiple distinct data types appear
- Message lists every declaration site: branch flag, type, line/col
- Only variables checked; function overloads share a signature-keyed namespace
  and are already covered by `Redefinition`

Observed on shipping shaders:
- `scene_ShadowMap` in `Shadow.glsl` — `sampler2DShadow` under
  `#ifdef GRAPHICS_API_WEBGL2`, `sampler2D` under `#else`. Deliberate — WebGL2
  wants hardware PCF, WebGL1 falls back to manual depth comparison. The
  conflict IS real per the "one name, one type" rule; the fix would be either
  renaming (`scene_ShadowMap_WGL2` / `scene_ShadowMap_WGL1`) or agreeing that
  this API-conditional pattern is exempt from the diagnostic.

Downstream classifications for the 75 other diagnostics on built-in shaders
(MissingReturn, AssignTypeMismatch, NonIndexableType, IndexOutOfBounds,
ConstructorArgCount) are NOT MacroBranchConflict — they're other analyzer
categories that need separate treatment (either analyzer improvements to
recognize legal GLSL patterns like `length(vec3) -> float`, or shader-level
fixes). Tracked separately.
… divergent types across `#if/#else`"

This reverts commit 3cdd448.
- SymbolInfo carries the `#ifdef` branch signature of its declaration;
  SymbolTable filters candidates by `isBranchVisibleFrom` vs a callsite branch
- TreeNode inherits a branch from its first terminal descendant;
  ASTNode.get() pushes it into SymbolTableStack._currentBranch so insert()
  stamps declarations with the right branch
- Reference lookups (VariableIdentifier, PostfixExpression.field,
  FunctionCallGeneric, ArraySpecifier size) opt in by passing `this._branch` —
  same/nested branch resolves, mutually-exclusive is invisible
- Duplicate / redefinition checks (SingleDeclaration, VariableDeclaration,
  FunctionDefinition) stay on the legacy `!isInMacroBranch` skip — otherwise
  the `#ifndef X_INCLUDED / #define X_INCLUDED / #endif` include-guard pattern
  would falsely trigger Redefinition on every guarded chunk
- Adds BranchAwareLookup.test.ts covering same-branch resolve, outer-scope
  visibility, and preserved errors inside macro branches
- variable_declaration reduces `fully_specified_type ID array_specifier`
  but SymbolType was constructed without the array specifier arg
- Every `float arr[N]` at pass-body scope stored as scalar `float`, so any
  `arr[i]` reference misfired NonIndexableType with base type 'float'
- Pull children[2] as ArraySpecifier when the production is length 3 and
  thread it through to the VarSymbol
- _blockGuaranteesReturn's CFG treated MacroIfStatement as opaque, so a
  function body ending in `#ifdef X return a; #else return b; #endif` fired
  MissingReturn even though every runtime arm returns
- Recurse into MacroIfStatement / MacroBranch mirroring the SelectionStatement
  handling: both `#if` and `#else` arms must guarantee for the block to count
- Bare `#endif` (no `#else`) stays conservative — runtime preprocessor may
  see zero arms match
- IndexOutOfBounds: skip the vector-size bounds check when the base is an
  array (`ivec2 arr[N]; arr[i]` indexes the outer array, not the inner
  ivec2). Array-size check still runs when the size is known.
- ConstructorArgCount: `matN(matM)` is legal per GLSL ES §5.4.3 (matrix
  truncated/padded diagonally). Short-circuit the exact-component-count
  check for single-matrix-arg matrix constructors before it fires on the
  source's total component count.
- AssignTypeMismatch: compound-op assign (`+=` `-=` `*=` `/=`) is
  `L = L op R`, not `L = R`. `vec3 *= float` is legal (scalar⊙vector
  broadcast); check assignability of arithmeticResultType(L, R) back to L.
  Operator token lives at AssignmentOperator.children[0], not
  AssignmentExpression.children[1] (that's the non-terminal wrapper).
- BuiltinFunction.resolveOverload locked the Size dimension from a scalar arg
  even when another arg was TypeAny, over-specializing calls like
  `max(TypeAny, 0.0)` to float. Track `sizeAmbiguous` / `scalarTypeAmbiguous`
  and fall through to TypeAny when the return family shares an ambiguous
  dimension.
- User-function overload resolution: `SymbolInfo.equal` treats TypeAny args
  as wildcards, so reverse-insertion lookup silently picks whichever overload
  was inserted last (e.g. shipping `permute` ships `float`/`vec3`/`vec4`
  variants; a TypeAny-typed arg matched all three but committed to the
  last-inserted return type). When multiple overloads match with divergent
  return types, keep `fnSymbol` set but drop the call's type to TypeAny.
- VariableIdentifier.lookupAll returns every branch-visible decl; if their
  identity (base type + isArray + arraySize) diverges, commit to TypeAny +
  isArray=false + arraySize=undefined instead of silently picking the
  last-inserted candidate (that was environment-sensitive flaky behaviour)
- Emits AmbiguousMacroBranchType at the reference site so callers know the
  checks are disabled; silent TypeAny would hide the fact
- Report-once per (pass, symbol name) — a divergent symbol may be
  referenced dozens of times (renderer_BlendShapeWeights has ~110 index
  sites across the include chain); flooding the editor buries the signal
- Symmetric with FunctionCallGeneric's existing overloadTypeAmbiguous guard
- Shipping shaders: 9 warnings across 4 symbols, 0 new error introduced
- Perf overhead measured at ~2% wall clock on parseShaderPass; codegen
  path is unchanged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant