refactor(shader): split parser/compiler/analyzer + structured diagnostics (RFC #3017)#3054
Draft
zhuxudong wants to merge 96 commits into
Draft
refactor(shader): split parser/compiler/analyzer + structured diagnostics (RFC #3017)#3054zhuxudong wants to merge 96 commits into
zhuxudong wants to merge 96 commits into
Conversation
- 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
- PostfixExpression index branch flags a non-integer index (e.g. v[1.5]) via ParserUtils.isIntegerType; integer indices fall through to the existing IndexOutOfBounds bounds check - AB test (err v[1.5] / ok v[1]) + coverage case
- FunctionCallGeneric builtin-constructor branch flags a sampler or struct argument: InvalidConversion for a single-arg cast, ConstructorArgType for a multi-arg constructor - AB tests (err float(sampler), vec2(sampler,..) / ok numeric ctor) + coverage
- a builtin vecN constructor flags too-few components from its arguments (scalar=1, vecN=N); a single-scalar splat and matrix/unknown args are skipped (conservative, no false positives) - add ParserUtils.isScalarType - AB test (err vec3(1.,2.) / ok splat + exact) + coverage case
- collect gl_Position references as a parse-time clue (mirrors gl_FragColor); ShaderIOAnalyzer flags a present vertex entry that writes gl_Position nowhere (global clue, so a single write clears it -> no false positive) - give IO-test shaders that omitted it a gl_Position write (valid except the rule under test) - AB test (err empty vert / ok writes gl_Position) + coverage case
- Multiplicative / Additive expressions deduce the result type via ParserUtils.arithmeticResultType (same type, or numeric-scalar x vector/matrix); every ambiguous case stays TypeAny, so it only adds type info and never mis-deduces (zero codegen regression) - this improves NoMatchingOverload / ReturnTypeMismatch / AssignTypeMismatch for arithmetic operands (the RFC's type-system-gated existing rules) - AB test (vec3 + vec3 -> vec3 enables a float-assign mismatch) + ok case
- StatementList flags the statement right after a terminal jump (return / break / continue / discard); a nested block / if / loop is not a direct jump, so code after a conditional return is not flagged (no false positive) - AB test (err return-then-stmt / ok conditional return) + coverage case
- FunctionDefinition walks its body once tracking loop depth (post-order reduction prevents reading the enclosing loop at the jump itself); break/continue at depth 0 is flagged, loop-nested jumps are not (no false positive) - AB test (err break in frag / ok break in for) + coverage case
- array-of-array is target-divergent (GLSL ES 3.00 / WGSL allow it, ES 1.00 doesn't) and the backend can always emit it, so the parser's target-agnostic ArrayOfArray check is removed -- it was non-fatally false-flagging valid ES300 shaders (e.g. Bloom/Uber, where it is macro-generated); codegen emits the declaration and the driver validates per target - drop the ArrayOfArray DiagnosticType + its coverage case
- NonIndexableType (Naga expression.rs InvalidBaseType): indexing a scalar non-array base. PostfixExpression unwraps `base[index]` to a bare variable; a scalar type with `!isArray` reports. Arrays and vectors are excluded. - VariableIdentifier now carries `isArray` (typeInfo alone dropped array-ness) set from the resolved symbol's dataType.arraySpecifier. - ExpectedSampler (Naga expression.rs ExpectedSamplerType): a texture-sampling builtin whose arg0 isn't a sampler. Checked in FunctionCallGeneric before the generic NoMatchingOverload fallback for the texture-family builtin names.
- NonFlatIntegerVarying (Naga interface.rs InvalidInterpolationForInteger): an integer-typed varying member without `flat`. GLSL ES gives integers no default interpolation, so `flat` is required. - StructProp now carries `isFlat`; StructDeclaration captures it from the `type_qualifier type_specifier struct_declarator_list` production (the grammar accepts `flat` on struct members). - ShaderIOAnalyzer flags integer non-flat members only for the Varying role; attribute/MRT roles are excluded by passing the role into _pushStruct.
- NonConstInitializer (Naga function.rs NonConstOrOverrideInitializer): a const variable whose initializer isn't compile-time constant. Checked at SingleDeclaration when the type qualifier is const. - NonConstArraySize (Naga expression.rs ConstExpr NonConstOrOverride): an array sized by a bare non-const variable. Checked at ArraySpecifier; literals and compound arithmetic are left alone so macro-sized arrays don't false-positive. - Shared const resolution: VarSymbol records isConst (from a const-qualified declaration); ParserUtils.isConstExpr accepts numeric literals, #define names, and identifiers bound to a const symbol. FullySpecifiedType exposes isConst.
- EntryNotFound: a bound entry name that resolves to no function (e.g. `VertexShader = vrt`). ShaderIOAnalyzer reports when the entry string is non-empty but _entryFns is empty; empty entries stay MissingEntry's job. - Entry-name source range is plumbed through: ShaderSourceParser stores the entry token location on IShaderPassSource (vertexEntryLocation / fragmentEntryLocation); ShaderAnalyzer threads it into ShaderIOAnalyzer.analyze. - Codegen / injection callers don't have the pass source, so the location is optional and falls back to a 0-position — the diagnostic still fires.
- extract ParserUtils.hasQualifier(node, keyword); drop the duplicate hasConstQualifier + StructDeclaration._hasFlatQualifier walkers - extract ParserUtils.firstNonArithmeticOperand; dedup the operand-validity check in Multiplicative/AdditiveExpression - flatten the PostfixExpression index branch with early returns - trim a WHAT comment in ShaderIOAnalyzer to WHY
- ConstDivideByZero: only flag integer div/mod by zero; float 1.0/0.0 is Inf, not an error (matches Naga validate_constant_divisor returning Ok for floats) - IndexOutOfBounds: bounds-check a constant index against a fixed-size array, not just a vector (Naga bounds-checks fixed-size arrays); VariableIdentifier now carries arraySize from the symbol's array specifier - isConstExpr: a macro use site lexes as MACRO_CALL, so its inner node is a MacroCallSymbol/Function, not a token — accept it so a #define-sized array no longer false-positives NonConstArraySize - tests: integer vs float div-by-zero AB, array index OOB AB, macro-sized array ok, continue-outside/inside-loop MisplacedControlFlow AB
- DiagnosticCoverage now asserts every DiagnosticType has a triggering test (the local cases + the codes covered in ShaderAnalyzer/ShaderIOAnalyzer); a future diagnostic shipped with no test anywhere fails this gate
- IShaderPassSource types it structurally (design stays class-free); the parser stored a ShaderRange there, so b:types (tsc) rejected the structural literal - b:module (SWC) skips type-checking, so this only surfaced under b:types
- not a Naga/GLSL rule and absent in dev/2.0; GLSL ES silently accepts code after return/break/continue, so the generated shader compiles either way - cosmetic lint, zero functional impact, zero hits across the shader corpus - removes the check + _isTerminalJump + enum member + 2 AB tests + coverage
- formatDiagnosticSource (shader-parser): full error span + contextLines
padding, gutter line numbers, carets; GSError.toString delegates to it
- formatDiagnostic(d) (shader-analyzer): the shared entry; _logDiagnostics
logs it, so WebGLEngine.create({shaderAnalyzer}) prints the same block
- contextLines fixed at 5 internally, not exposed as user config
- InvalidVaryingStruct + InvalidAttributeStruct + InvalidMrtStruct -> InvalidIOStruct (Naga has one VaryingError::InvalidType; role stays in the message) - VertexEntryReturnType + FragmentEntryReturnType -> InvalidEntryReturnType (Naga EntryPointError::Result(VaryingError); stage stays in the message) - InvalidConversion folded into ConstructorArgType (Naga ComposeError::ComponentType covers single- and multi-arg) - DiagnosticType 48 -> 44; tests, coverage gate and example updated
- dat.gui dropdown over all 44 DiagnosticType codes loads a triggering shader - right panel reuses the built-in formatDiagnostic (line-numbered context + carets) - editable textarea + line-number gutter; re-analyzes live
- the check already allows void (void frag + gl_FragColor is the classic GLSL ES style); the InvalidEntryReturnType message wrongly omitted void
- ReturnTypeMismatch + ReturnInVoidFunction -> InvalidReturnType (Naga has one FunctionError::InvalidReturnType; the two sites keep their distinct messages) - DiagnosticType 44 -> 43; tests, coverage gate and example updated
- move the pure type-value logic (predicates, deduce, compat, typeName) into a dedicated TypeSystem module; ParserUtils keeps AST/grammar/macro/eval helpers - the IR's type semantics now live in one identifiable place - pure refactor, no behavior change (313 green, codegen byte-identical)
… parser (B0) - new ShaderValidator: a post-parse pass over the typed AST — the nucleus of a real validator, toward Naga's build / validate / emit separation - move NonBoolCondition out of the parser's inline semanticAnalyze as the proof; the parser keeps model-building, validation moves to the analyzer pass - the analyzer runs it in both analyze() and the injected _diagnose() path - 313 green, codegen byte-identical (the moved check was validation-only)
- move 6 stateless validation checks off the parser's inline semanticAnalyze into the validator pass: ConstructorArgType/Count, InvalidUnaryOperand, InvalidBinaryOperands, ConstDivideByZero, ShiftOutOfRange. Type inference (the arithmeticResultType deduce, ShiftExpression.type) stays inline. - ExpectedSampler kept inline: its short-circuit return suppresses NoMatchingOverload for the same call (parse-order context) — moves later with the overload checks. - 313 green, codegen byte-identical
…(B1b) - move IndexOutOfBounds, NonIntegerIndex, NonIndexableType, InvalidSwizzle and GlFragData (PostfixExpression) + NonConstructibleReturnType into the validator - PostfixExpression.semanticAnalyze keeps only the struct.field path (symbol-table dependent); NonConstInitializer/NonConstArraySize deferred (need const-eval ctx) - 313 green, codegen byte-identical
- the validator walk now threads { currentFunction, loopDepth }, so checks that
need the enclosing function / loop depth move off the parser:
InvalidReturnType, MissingReturn, MisplacedControlFlow, RecursiveFunction
- removes FunctionDefinition._checkControlFlow (the validator's loopDepth subsumes
it); parser keeps model-building (returnStatement / curFunctionInfo recording)
- order/overload-coupled checks (ExpectedSampler, UndefinedFunction,
NoMatchingOverload, UseBeforeDeclaration, Redefinition) stay inline by design
- 313 green, codegen byte-identical
- ShaderValidator is instance-based now: source/errors are fields instead of threaded through every _walk/_check* method (static validate() entry unchanged) - multiplicative nodes scan operands once: _checkArithmeticOperands returns whether it reported, _checkConstDivideByZero gates on the operator before touching operands - narrow createGSError to return GSError (it always did), dropping the <GSError> casts in ShaderValidator and ShaderIOAnalyzer - drop dead TypeSystem.typeCompatible and a redundant optional-chain in _checkJump - 313 green, codegen byte-identical
B2a recorded returnStatement unconditionally so the validator could diagnose a void function returning a value. But the fragment-entry rewrite in GLES100/300 reads returnStatement to turn a fragment return into a `gl_FragColor = <expr>` assignment — it needs children[1] to be an expression, which is only guaranteed when the function is non-void. A void frag() with `return;` early-exits would emit malformed GLSL. - restore the void gate in FunctionDefinition.semanticAnalyze (matches dev/2.0 byte-for-byte for the built-in shaders) - validator reconstructs the void-with-value signal from walk context in _checkJump instead of leaning on the parser field; _checkFunctionReturn keeps only the MissingReturn branch - regression test: void frag with early return emits well-formed GLSL, no `gl_FragColor = ;` garbage
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景与目标
dev/2.0 的 shader 诊断与运行时编译耦合在一包,靠
#if _VERBOSE门控:release 产物零诊断、错误无行列定位、同一错误多次上报、外部无法消费。本 PR 把 shader 体系拆为三包并引入结构化诊断:
架构
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 anashader-compiler与shader-analyzer互不依赖,是 parser 之上两个平等消费者。类型/控制流校验抽成独立的ShaderValidatorpass(走 typed-AST),类型语义收口到TypeSystem;与解析序耦合的校验(重载 / 声明序 / const 求值)+ 管线 IO + RenderState 仍在 parser 内联打标。整体对齐 Naga「中性源 + 独立 Validator + 多 backend」的分离方向(建模型 / 校验 / 发射);模型是带注解的 typed-AST,非完全 lower 的 IR。核心机制:校验独立于 codegen
reportError+#if _VERBOSEShaderValidator走 typed-AST 现算;解析序耦合项 parser 内联打标(error-as-clue)ShaderValidator走 typed-AST 做类型/控制流校验 + 收集 parser 线索 →Diagnostic[]对外 API
analyzer.analyze(src)Diagnostic[]WebGLEngine.create({ shaderCompiler, shaderAnalyzer })formatDiagnostic(d)诊断能力
43 个
DiagnosticType,逐条对 Nagavalid/源码核过粒度(过度拆分已合并、ShaderLab/GLSL 专属项标注依据);完整目录 + 正/误示例见 RFC #3017 §3。按实体分类:优势
file:line:col [Code] msg、LLM 拿去自动修复。测试
DiagnosticType+ 位置)。IShaderProgramSource字节对等(precompile 守护)。DiagnosticType必有触发用例,缺测即 fail。npm run build(b:module+b:types,tsc 全量类型检查)后npm run test。