diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index da151816c..d1d30b268 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -734,6 +734,7 @@ The Direct2D authoring projects were removed before the first release and restor | Generated HLSL, a variable a static field initializer would have to declare | Writes an out argument into a call in a static field initializer as the author wrote it, declaration and all, so the generated HLSL declares a variable inside an argument list and the shader compiler rejects it while naming generated code. A discarded out argument answers the same way, the discard reaching the generated HLSL as an identifier nothing declares. The shader body declares the variable at the start of the body and passes it to the call, so where the author writes the call decides whether it works | Reports `CMPW0126`, or `CMPWD2D0098` on the Direct2D path, at the declaration and at the discard alike. The two reporting sites are the two places the rewriter for a body introduces such a variable, so what is refused is what that rewriter hoists rather than the shapes that happened to be measured. Giving the variable a global static of its own instead was rejected: one storage would be shared by every invocation of the shader, and HLSL leaves the order of global static initializers undefined, which is the hazard `CMPW0124` already refuses. The report names the source the author wrote, where the failure it replaces named the shader type and quoted a line of generated HLSL | `121de6e8` | | Generated HLSL, a static field of the shader reaching itself | Rewrites the initializer of a static field of the shader without claiming an entry for it, the collection the other rewritings claim in holding the fields of other types, and leaves a static method of the shader for the generator to write out, which it does before it rewrites any initializer. An initializer that reaches the field it initializes therefore finds nothing on any of its routes: nothing is reported, the generator does not fault, and the shader compiler accepts generated HLSL that reads a global through a function reading it back, so the shader computes a value C# never produces. Reading the field by its own name and reading it through the shader type differ only in that the second writes the type name into the generated HLSL, which the compiler then rejects while naming generated code | Reports the read from a walk over what every initializer reaches, run once after the initializers are rewritten, which the row on a static field accessed before its initializer has run describes. Nothing is claimed for a field of the shader and nothing is reported as an initializer is rewritten: a rewriting does not pass through every declaration an initializer reaches, a declaration being imported once and a method of the shader never, which is what left routes unanswered while the read was reported where it was rewritten. Before the walk, the read was answered where it was written, by name alone in the shared rewriter and through the shader type in each of the two rewriters, and a call to a static method of the shader was walked from the rewriting site, which reached neither a declaration the body or an earlier initializer had imported first, nor a declaration in the middle of its own import, nor a field of another type the walk found. Two fields whose initializers reach each other through a static method are answered as a read C# performs too early rather than as a cycle, C# running the first initializer to completion before the second starts. The type symbol for the shader moves to the type both rewriters derive from, the shared logic having to tell a member of the shader from a member of any other type, and each of the two having held its own copy | `41027c2f`, `18e3c03d`, `6d4e2354` | | Generated HLSL, a static field accessed before its initializer has run | Writes an access to a static field into the generated HLSL as it stands wherever the access is performed, and writes the static fields out in an order derived from what each initializer reached. C# runs the static field initializers of a type once, in the order the fields are declared, and a field of that type whose turn has not come yet holds the default value of its type: a read of it is that default, and a write to it is discarded when its initializer runs, whether the initializer reaches the field directly, through a method of the shader, through an imported declaration, or from the initializer of an imported field. The generated HLSL reproduces neither: the shader compiler folds the initializer the later field carries when it can, so the shader computes the initialized value where C# computes the default and keeps a write C# discards, leaves the read undefined otherwise, and does not compile the direct read at all, naming a global declared later in generated code. A field read through a function before its initializer has run was measured to compute 10 on DXC and FXC alike where C# computes 0, with neither compiler reporting anything, and a write through a function to a later field carrying a constant initializer was measured to stay on DXC where C# computes the initializer's value, FXC refusing that write while naming generated code. An initializer reaching back into the field it initializes is the same read, and the cycle rows above answered it only where a rewriting passed through the read while the field was claimed | Reports `CMPW0124`, or `CMPWD2D0096` on the Direct2D path, at the access, naming the field accessed and the initializer that performs the access too early. The report comes from one walk over every static field the generated HLSL declares, the shader's own and the imported ones alike, run once after the initializers are rewritten: from each initializer it follows the method, local function or constructor a call resolves to and the initializer of a static field of another type, whose initializers C# runs when that type is first touched, and reports a read or a write of a field of the initializer's own type whose turn has not come and that carries an initializer, the field being initialized among them for a read. A field carrying no initializer holds the default value of its type in C# and in the generated HLSL alike, and a write to the field being initialized is overwritten by its initializer in both, so neither is reported; both were measured on DXC. A write is a simple assignment to the field or an out argument, and every other reference reads the field, a compound assignment and a ref argument among them. A field of the same type is not walked into, its initializer having run already or not running until its own turn, a local function is walked only through a call, C# not running one that is never called, and an access two initializers both perform too early is reported once, for the first of the two in declaration order, so every access is one report. The walk does not follow the order of the statements it passes, so a read after a write in the same declaration is reported like any other. The same descriptor answers for an initializer reaching itself, that being the first field whose turn has not come, so the identifier the earlier rows introduced widens rather than a second one being added, and its message names both fields. The walk resolves semantic information only for the kinds an access, a call or a construction can be written as, which is the pre-filter the rewriting uses too | `6d4e2354` | +| Generated HLSL, a static field assigned by a static constructor | Imports a static field by rewriting its initializer alone, and writes no static constructor into the generated HLSL. C# runs the static constructor of a type when the type is first touched, after the initializers of its static fields, so a static field the constructor assigns holds that value from then on, whereas the generated HLSL declares the field with the value of its initializer, or with none, which both shader compilers read as zero without reporting anything. A `readonly` field assigned only by the constructor was measured to compute 5 in C# and 0 in the generated HLSL on DXC and FXC alike, and a mutable one the same | Reports `CMPW0130`, or `CMPWD2D0103` on the Direct2D path, at the write, naming the field and the type whose static constructor performs it. The report comes from a walk over the static constructor of every type the rewriting touched, which are the shader type and the types of every imported method, constructor and static field, each followed through the method, local function or constructor a call resolves to, the way the walk of the row above follows an initializer, the two sharing that walk. A write to any static field the generated HLSL declares is reported, whichever of those types the constructor belongs to, C# running that constructor when its type is touched and the generated HLSL never; a read is not, and neither is a write to a field the generated HLSL does not declare. A write is an assignment of any kind, an increment or a decrement, or a `ref` or `out` argument, a `ref` one counting because the callee is free to write through it. A write two constructors both reach is reported once, for the first of the two in the order the types were touched | `af839246`, `cf1905fe` | | Generated HLSL, the requirements a shader raises | Keeps what a rewriting gathers on the rewriter that gathered it, and carries it out by hand wherever one rewriter creates another. Which of those places carry anything is left to a hook the specialized rewriter may implement, and a hook nothing implements is silent. On the Direct2D path the scene position requirement is raised inside the rewriter for a called method, an instance method, a constructor, the initializer of an external static field, an initializer of the shader itself, and a declaration imported by one, and reaches the generator from none of them, so `CMPWD2D0045` answers only for a call written in the shader body. The generated HLSL then reads the position without the define its header needs, and the author is left with the failure FXC raises against generated code, which names a line of generated source and not the attribute to add. On the compute path the three initializer routes lose the requirement that a dispatch cover whole thread groups, so a range that cuts a group in half is accepted for a shader that waits for one. Two of the hooks are declared and implemented with no call site anywhere in the tree | Holds the requirements in one instance per shader, handed to every rewriter created for it, so a requirement is raised where it is found and read where the shader is written, with no path in between having to carry it. Which requirements exist still differs between the two generators, so they are declared in a partial declaration each one carries, the way the rewriters themselves already are, and a rewriter that is not handed them does not compile. The two hooks with no call site are removed along with the carrying they existed for, and so is the suppression that hid them: measured over both generators, that rule had nothing else to report, so a hook declared and never called now stops the build. The other rule in that suppression is kept, one field being assigned and never read where the shared file compiles into the Direct2D generator, and what it hides is written beside it. The hook that raises a requirement from a mapped call moves to the type both rewriters derive from: what a call requires does not depend on whether it was written in a body or in an initializer, and declaring it twice was the shape that left one of the two uncalled. The sampler requirement is left where it is, a texture being refused as a static field and as a parameter alike, so only a rewriting of the shader's own members can raise it, which was measured rather than read. Measured on both paths over the six places a rewriter creates another one and the one each generator creates for a static field of the shader itself | `44f39c86`, `6b8f4af7`, `f937b857`, `33756710` | | Generated HLSL, primary constructors | Refuses the construction of a type declaring a primary constructor, which is deliberate, the captures of such a constructor reaching the members of the type in ways the rewriting cannot follow. It reports the diagnostic for a constructor with no source to analyze, because the check that reaches it is whether a constructor declaration can be found, and a primary constructor has none of its own. The author reads a reason that is visibly not the one, the source being right there, and the same refusal covers a constructor from another assembly, for which the reason is correct | Reports `CMPW0120`, or `CMPWD2D0093` on the Direct2D path, when the constructor is declared in source, and keeps the older diagnostic for one that is not. The two are told apart by whether the constructor has any declaring syntax reference at all, a primary constructor having one on the type declaration. The refusal itself is unchanged, and so is the shader type's own primary constructor, whose captures become the shader fields | `00f95213` | | Generated HLSL, generic local functions | Lifts a local function to a top level HLSL function whether or not it is called, carrying its type parameter list with it, and asks whether a method is generic only where a call is rewritten. A generic local function that is never called reaches the HLSL compiler with `` on it, and one that is called is reported against the call rather than against the declaration the author has to change | The declaration is refused, so the report names it, and the lifted function, which still carries its type parameter list, never reaches the shader compiler. The call site leaves a local function alone, because the declaration has already answered and two reports would name two places for one cause | `e364c7e4` | diff --git a/README.ja.md b/README.ja.md index b37b2035d..e0c2372da 100644 --- a/README.ja.md +++ b/README.ja.md @@ -260,7 +260,7 @@ GPUが書いたバッファを、以降のシェーダーへ読み取り専用 ### コンパイル時の検証 -以上の宣言はアナライザーが検査し、接頭辞 `CMPW` の診断113種類として報告します。対象は属性の位置、ホストとパイプラインメソッドの形、スロットの宣言、資源の契約、生成されるオーバーロードの衝突です。一部にはコード修正が付きます。 +以上の宣言はアナライザーが検査し、接頭辞 `CMPW` の診断114種類として報告します。対象は属性の位置、ホストとパイプラインメソッドの形、スロットの宣言、資源の契約、生成されるオーバーロードの衝突です。一部にはコード修正が付きます。 --- @@ -297,7 +297,7 @@ ReadOnlyMemory bytecode = D2D1PixelShader.LoadBytecode() ReadOnlyMemory buffer = D2D1PixelShader.GetConstantBuffer(new DifferenceEffect(1)); ``` -これらの宣言は、`CMPWD2D` を接頭辞とする102件の診断を報告するアナライザーが検査します。シェーダーは Direct2D が受け付ける DXBC へ FXC でコンパイルします。`d3dcompiler_47.dll` は Windows に同梱されているため、このパッケージはコンパイラーを同梱しません。 +これらの宣言は、`CMPWD2D` を接頭辞とする103件の診断を報告するアナライザーが検査します。シェーダーは Direct2D が受け付ける DXBC へ FXC でコンパイルします。`d3dcompiler_47.dll` は Windows に同梱されているため、このパッケージはコンパイラーを同梱しません。 --- diff --git a/README.md b/README.md index b0e6413c1..84cb56acb 100644 --- a/README.md +++ b/README.md @@ -260,7 +260,7 @@ The budget covers the resources the device creates itself. A device using an all ### Compile-time validation -The declarations above are checked by analyzers that report 113 diagnostics with the `CMPW` prefix, covering attribute placement, host and pipeline method shape, slot declaration, resource contracts and generated overload conflicts. Some carry a code fix. +The declarations above are checked by analyzers that report 114 diagnostics with the `CMPW` prefix, covering attribute placement, host and pipeline method shape, slot declaration, resource contracts and generated overload conflicts. Some carry a code fix. --- @@ -297,7 +297,7 @@ ReadOnlyMemory bytecode = D2D1PixelShader.LoadBytecode() ReadOnlyMemory buffer = D2D1PixelShader.GetConstantBuffer(new DifferenceEffect(1)); ``` -The declarations are checked by analyzers that report 102 diagnostics with the `CMPWD2D` prefix. Shaders are compiled to DXBC with FXC, which is what Direct2D accepts; `d3dcompiler_47.dll` ships with Windows, so the package bundles no compiler of its own. +The declarations are checked by analyzers that report 103 diagnostics with the `CMPWD2D` prefix. Shaders are compiled to DXBC with FXC, which is what Direct2D accepts; `d3dcompiler_47.dll` ships with Windows, so the package bundles no compiler of its own. --- diff --git a/src/ComputeWeave.D2D1.SourceGenerators/AnalyzerReleases.Unshipped.md b/src/ComputeWeave.D2D1.SourceGenerators/AnalyzerReleases.Unshipped.md index f93364086..5d116a197 100644 --- a/src/ComputeWeave.D2D1.SourceGenerators/AnalyzerReleases.Unshipped.md +++ b/src/ComputeWeave.D2D1.SourceGenerators/AnalyzerReleases.Unshipped.md @@ -7,3 +7,4 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- CMPWD2D0101 | ComputeWeave.D2D1.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) CMPWD2D0102 | ComputeWeave.D2D1.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) +CMPWD2D0103 | ComputeWeave.D2D1.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) diff --git a/src/ComputeWeave.D2D1.SourceGenerators/D2DPixelShaderDescriptorGenerator.HlslSource.cs b/src/ComputeWeave.D2D1.SourceGenerators/D2DPixelShaderDescriptorGenerator.HlslSource.cs index b06a68c00..9005683c3 100644 --- a/src/ComputeWeave.D2D1.SourceGenerators/D2DPixelShaderDescriptorGenerator.HlslSource.cs +++ b/src/ComputeWeave.D2D1.SourceGenerators/D2DPixelShaderDescriptorGenerator.HlslSource.cs @@ -346,6 +346,7 @@ private static ImmutableArray GetStaticFields( CancellationToken token) { using ImmutableArrayBuilder declared = new(); + List declaredFields = []; foreach (ISymbol memberSymbol in structDeclarationSymbol.GetMembers()) { @@ -378,6 +379,7 @@ private static ImmutableArray GetStaticFields( typeDeclaration, assignmentExpression, HlslDefinitionsSyntaxProcessor.GetStaticFieldOrder(staticFieldDefinitions))); + declaredFields.Add(fieldSymbol); // An initializer may import a method that declares a local function, which HLSL cannot // nest, so the ones lifted out of it are carried up to be written like any other @@ -397,6 +399,18 @@ private static ImmutableArray GetStaticFields( diagnostics, token); + // A static field a static constructor assigns is reported (same as in the DX12 generator) + HlslDefinitionsSyntaxProcessor.ReportStaticFieldsAssignedByAStaticConstructor( + structDeclarationSymbol, + declaredFields, + staticMethods, + instanceMethods, + constructors, + staticFieldDefinitions, + semanticModel, + diagnostics, + token); + // One sequence ordered by when each field finished (same as in the DX12 generator) HlslStaticField[] fields = [.. declared.WrittenSpan, .. staticFieldDefinitions.Values]; diff --git a/src/ComputeWeave.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/src/ComputeWeave.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index 36fb78f8c..0a8b8ba81 100644 --- a/src/ComputeWeave.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/src/ComputeWeave.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -1462,6 +1462,22 @@ partial class DiagnosticDescriptors description: "C# runs the static field initializers of a type once, in the order the fields are declared. A field whose turn has not come yet holds the default value of its type: a read of it is that default, and a write to it is discarded when its initializer runs. Both happen when an initializer reaches a field of the same type declared after it, directly or through any declaration it reaches, and the read happens when an initializer reaches back into the field it initializes. The generated HLSL reproduces neither: the shader compiler folds the initializer of a constant declared later, so the read is the initialized value, and refuses the other forms, so the shader either computes a value C# never produces or fails against generated code the author never wrote. A field carrying no initializer holds the default value in both and is not reported, and neither is a write to the field being initialized, which its initializer overwrites in both. Before this report an initializer reaching itself faulted the generator instead, adding the same key to the collection of static field definitions twice, which discards the descriptors for every shader in the compilation unit and leaves the author with errors that name none of this.", helpLinkUri: "https://github.com/routersys/ComputeWeave"); + /// + /// Gets a for a static field assigned by a static constructor. + /// + /// Format: "The static field {0} is assigned by the static constructor of {1}, which the generated HLSL does not run (the field holds the value of its initializer there, or zero without one)". + /// + /// + public static readonly DiagnosticDescriptor StaticFieldAssignedByStaticConstructor = new( + id: "CMPWD2D0103", + title: "Static field assigned by a static constructor", + messageFormat: "The static field {0} is assigned by the static constructor of {1}, which the generated HLSL does not run (the field holds the value of its initializer there, or zero without one)", + category: "ComputeWeave.D2D1.Shaders", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "C# runs the static constructor of a type when the type is first touched, after the initializers of its static fields, and a static field the constructor assigns holds that value from then on. The generated HLSL runs no static constructor and declares the field with the value of its initializer, or with no initializer, which the shader compiler reads as zero, so the shader computes a value C# never produces and neither compiler reports anything. The static constructors of the shader type and of every type an imported declaration belongs to are followed through every declaration they reach, and a write to any static field the generated HLSL declares is reported, whichever of those types the constructor belongs to.", + helpLinkUri: "https://github.com/routersys/ComputeWeave"); + /// /// Gets a for an operation whose operands C# widens past the HLSL type set. /// diff --git a/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxProcessors/HlslDefinitionsSyntaxProcessor.cs b/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxProcessors/HlslDefinitionsSyntaxProcessor.cs index 2d7c5b8ae..82005cac5 100644 --- a/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxProcessors/HlslDefinitionsSyntaxProcessor.cs +++ b/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxProcessors/HlslDefinitionsSyntaxProcessor.cs @@ -439,21 +439,19 @@ public static int GetStaticFieldOrder(IDictionary /// /// /// The walk starts from the initializer of every static field the generated HLSL declares, the ones of the - /// shader and the imported ones alike, and follows every declaration it reaches: the method, local function - /// or constructor a call resolves to, and the initializer of a static field of another type, whose + /// shader and the imported ones alike, and follows every declaration it reaches (see + /// ) and the initializer of a static field of another type, whose /// initializers C# runs when that type is first touched. A field of the same type is not walked into, its - /// initializer having run already or not running until its own turn, and a local function is walked only - /// through a call, C# not running one that is never called. An access two initializers both perform too - /// early is one place to change, so it is reported once, for the first of the two in declaration order. - /// The walk does not follow the order of the statements it passes, so a read after a write in the same - /// declaration is reported like any other. + /// initializer having run already or not running until its own turn. An access two initializers both + /// perform too early is one place to change, so it is reported once, for the first of the two in + /// declaration order. The walk does not follow the order of the statements it passes, so a read after a + /// write in the same declaration is reported like any other. /// /// /// This runs once after the initializers are rewritten rather than as they are, because a rewriting does /// not pass through every declaration an initializer reaches: a declaration is imported once, so one the /// body or an earlier initializer imported is not rewritten again, and a method of the shader is never - /// imported. Semantic information is resolved only for the kinds an access, a call or a construction can - /// be written as, an access written as a member access resolving on the access alone. + /// imported. /// /// public static void ReportStaticFieldAccessesBeforeInitialization( @@ -492,17 +490,6 @@ public static void ReportStaticFieldAccessesBeforeInitialization( return order; } - // A simple assignment to the field and an out argument write it without reading it - static bool IsWrite(IFieldReferenceOperation reference) - { - return reference.Parent switch - { - ISimpleAssignmentOperation assignment => ReferenceEquals(assignment.Target, reference), - IArgumentOperation { Parameter.RefKind: RefKind.Out } => true, - _ => false - }; - } - // The imported fields are walked in the declaration order of their type too, so that an access two of // them perform too early names the first of the two HashSet importedFields = new(staticFieldDefinitions.Keys, SymbolEqualityComparer.Default); @@ -525,66 +512,212 @@ static bool IsWrite(IFieldReferenceOperation reference) HashSet pending = new( GetInitializers(field.ContainingType).SkipWhile(candidate => !SymbolEqualityComparer.Default.Equals(candidate.Field, field)).Select(candidate => candidate.Field), SymbolEqualityComparer.Default); - HashSet visited = new(SymbolEqualityComparer.Default); - - WalkReachedNodes(initializer); - void WalkReachedDeclaration(ISymbol symbol, SyntaxNode? root) + ReachedDeclarationWalker walk = new(semanticModel, token, (walker, node, reference) => { - if (root is not null && visited.Add(symbol)) + IFieldSymbol reachedField = reference.Field; + + if (pending.Contains(reachedField)) { - WalkReachedNodes(root); + // A write to the field being initialized is overwritten by its initializer in the + // generated HLSL as well, so it is the one access that is not reported + if ((!IsOnlyWritten(reference) || !SymbolEqualityComparer.Default.Equals(reachedField, field)) && + reportedAccesses.Add(node)) + { + diagnostics.Add(StaticFieldAccessedBeforeInitialization, node, reachedField, field); + } + } + else if (!SymbolEqualityComparer.Default.Equals(reachedField.ContainingType, field.ContainingType) && + reachedField.TryGetSyntaxNode(token, out VariableDeclaratorSyntax? reachedDeclarator)) + { + walker.Walk(reachedField, reachedDeclarator.Initializer?.Value); } + }); + + walk.WalkNodes(initializer); + } + } + + /// + /// Reports every static field the generated HLSL declares that a static constructor C# would run assigns. + /// + /// The type symbol for the shader type. + /// The static fields of the shader type the generated HLSL declares. + /// The collection of discovered static methods. + /// The collection of discovered instance methods for custom struct types. + /// The collection of discovered constructors for custom struct types. + /// The collection of discovered static field definitions. + /// The instance for the type to process. + /// The collection of produced instances. + /// The used to cancel the operation, if needed. + /// + /// + /// C# runs the static constructor of a type when the type is first touched, after the initializers of its + /// static fields, and a static field the constructor assigns holds that value from then on. The generated + /// HLSL runs no static constructor: a static field holds the value of its initializer there, or zero + /// without one, so a field a static constructor assigns computes a value C# never produces. + /// + /// + /// The static constructors walked are those of every type the shader touches, which are the shader type + /// and the types of every imported declaration, and each is followed through every declaration it reaches + /// (see ). A write to any static field the generated HLSL declares is + /// reported, whichever type the constructor belongs to, C# running that constructor when its type is + /// touched and the generated HLSL never. A read is not, the constructor leaving the field as it was. A + /// write two constructors both reach is reported once, for the first of the two in the order the types + /// were touched. + /// + /// + public static void ReportStaticFieldsAssignedByAStaticConstructor( + INamedTypeSymbol structDeclarationSymbol, + IEnumerable declaredFields, + IDictionary staticMethods, + IDictionary instanceMethods, + IDictionary constructors, + IDictionary staticFieldDefinitions, + SemanticModelProvider semanticModel, + ImmutableArrayBuilder diagnostics, + CancellationToken token) + { + HashSet declaredStaticFields = new(declaredFields.Concat(staticFieldDefinitions.Keys), SymbolEqualityComparer.Default); + HashSet reportedWrites = []; + + // The types the shader touches, in the order the rewriting reached them, the shader type first + HashSet touchedTypes = new(SymbolEqualityComparer.Default); + List touchedTypeOrder = []; + + List members = [structDeclarationSymbol, .. staticMethods.Keys, .. instanceMethods.Keys, .. constructors.Keys, .. staticFieldDefinitions.Keys]; + + foreach (ISymbol member in members) + { + INamedTypeSymbol type = member as INamedTypeSymbol ?? member.ContainingType; + + if (touchedTypes.Add(type)) + { + touchedTypeOrder.Add(type); } + } - void WalkReachedNodes(SyntaxNode root) + foreach (INamedTypeSymbol type in touchedTypeOrder) + { + foreach (IMethodSymbol constructor in type.StaticConstructors) { - // The initializer can be the access or the call itself, so the root counts as a reached node too. - // A local function is entered from a call to it, the way a method is, rather than from the body - // it is declared in, that body not running it unless it calls it - foreach (SyntaxNode node in root.DescendantNodesAndSelf(descendant => descendant is not LocalFunctionStatementSyntax || ReferenceEquals(descendant, root))) + token.ThrowIfCancellationRequested(); + + if (!constructor.TryGetSyntaxNode(token, out ConstructorDeclarationSyntax? declaration)) { - token.ThrowIfCancellationRequested(); + continue; + } - if (node is not - (IdentifierNameSyntax or - MemberAccessExpressionSyntax or - InvocationExpressionSyntax or - BaseObjectCreationExpressionSyntax)) + ReachedDeclarationWalker walk = new(semanticModel, token, (_, node, reference) => + { + if (declaredStaticFields.Contains(reference.Field) && + IsWritten(reference) && + reportedWrites.Add(node)) { - continue; + diagnostics.Add(StaticFieldAssignedByStaticConstructor, node, reference.Field, type); } + }); - switch (semanticModel.For(node).GetOperation(node, token)) - { - case IFieldReferenceOperation { Field: { IsStatic: true, IsConst: false } reachedField } reference: - if (pending.Contains(reachedField)) - { - // A write to the field being initialized is overwritten by its initializer in - // the generated HLSL as well, so it is the one access that is not reported - if ((!IsWrite(reference) || !SymbolEqualityComparer.Default.Equals(reachedField, field)) && - reportedAccesses.Add(node)) - { - diagnostics.Add(StaticFieldAccessedBeforeInitialization, node, reachedField, field); - } - } - else if (!SymbolEqualityComparer.Default.Equals(reachedField.ContainingType, field.ContainingType) && - reachedField.TryGetSyntaxNode(token, out VariableDeclaratorSyntax? reachedDeclarator)) - { - WalkReachedDeclaration(reachedField, reachedDeclarator.Initializer?.Value); - } + walk.Walk(constructor, declaration); + } + } + } - break; - case IInvocationOperation { TargetMethod: { MethodKind: MethodKind.LocalFunction } localFunction }: - WalkReachedDeclaration(localFunction, localFunction.TryGetSyntaxNode(token, out LocalFunctionStatementSyntax? localFunctionStatement) ? localFunctionStatement : null); - break; - case IInvocationOperation { TargetMethod: { } method }: - WalkReachedDeclaration(method, method.TryGetSyntaxNode(token, out MethodDeclarationSyntax? methodDeclaration) ? methodDeclaration : null); - break; - case IObjectCreationOperation { Constructor: { } constructor }: - WalkReachedDeclaration(constructor, constructor.TryGetSyntaxNode(token, out ConstructorDeclarationSyntax? constructorDeclaration) ? constructorDeclaration : null); - break; - } + /// + /// Checks whether a reference to a field writes it without reading it. + /// + /// The instance to check. + /// Whether is the target of a simple assignment or an argument. + private static bool IsOnlyWritten(IFieldReferenceOperation reference) + { + return reference.Parent switch + { + ISimpleAssignmentOperation assignment => ReferenceEquals(assignment.Target, reference), + IArgumentOperation { Parameter.RefKind: RefKind.Out } => true, + _ => false + }; + } + + /// + /// Checks whether a reference to a field writes it, whether or not it reads it as well. + /// + /// The instance to check. + /// Whether is the target of an assignment, an increment or a decrement, or a or argument. + private static bool IsWritten(IFieldReferenceOperation reference) + { + return reference.Parent switch + { + IAssignmentOperation assignment => ReferenceEquals(assignment.Target, reference), + IIncrementOrDecrementOperation increment => ReferenceEquals(increment.Target, reference), + IArgumentOperation { Parameter.RefKind: RefKind.Out or RefKind.Ref } => true, + _ => false + }; + } + + /// + /// A walk over what a declaration reaches: the method, local function or constructor each call in it + /// resolves to, each declaration once, and whatever the visitor walks further. + /// + /// The instance for the type to process. + /// The used to cancel the operation, if needed. + /// Called with the walker, the node and the reference for every static, non constant field a reached node references. + /// + /// The root counts as a reached node too, an initializer being able to be the access or the call itself. A + /// local function is entered from a call to it, the way a method is, rather than from the body it is + /// declared in, that body not running it unless it calls it. Semantic information is resolved only for the + /// kinds an access, a call or a construction can be written as, an access written as a member access + /// resolving on the access alone. + /// + private sealed class ReachedDeclarationWalker(SemanticModelProvider semanticModel, CancellationToken token, Action visitFieldReference) + { + private readonly HashSet visited = new(SymbolEqualityComparer.Default); + + /// + /// Walks a declaration, unless it was walked already or has no syntax to walk. + /// + /// The symbol for the declaration. + /// The syntax of the declaration, if it has any. + public void Walk(ISymbol symbol, SyntaxNode? root) + { + if (root is not null && this.visited.Add(symbol)) + { + WalkNodes(root); + } + } + + /// + /// Walks the nodes of a body and every declaration they reach. + /// + /// The body to walk. + public void WalkNodes(SyntaxNode root) + { + foreach (SyntaxNode node in root.DescendantNodesAndSelf(descendant => descendant is not LocalFunctionStatementSyntax || ReferenceEquals(descendant, root))) + { + token.ThrowIfCancellationRequested(); + + if (node is not + (IdentifierNameSyntax or + MemberAccessExpressionSyntax or + InvocationExpressionSyntax or + BaseObjectCreationExpressionSyntax)) + { + continue; + } + + switch (semanticModel.For(node).GetOperation(node, token)) + { + case IFieldReferenceOperation { Field: { IsStatic: true, IsConst: false } } reference: + visitFieldReference(this, node, reference); + break; + case IInvocationOperation { TargetMethod: { MethodKind: MethodKind.LocalFunction } localFunction }: + Walk(localFunction, localFunction.TryGetSyntaxNode(token, out LocalFunctionStatementSyntax? localFunctionStatement) ? localFunctionStatement : null); + break; + case IInvocationOperation { TargetMethod: { } method }: + Walk(method, method.TryGetSyntaxNode(token, out MethodDeclarationSyntax? methodDeclaration) ? methodDeclaration : null); + break; + case IObjectCreationOperation { Constructor: { } constructor }: + Walk(constructor, constructor.TryGetSyntaxNode(token, out ConstructorDeclarationSyntax? constructorDeclaration) ? constructorDeclaration : null); + break; } } } diff --git a/src/ComputeWeave.SourceGenerators/AnalyzerReleases.Unshipped.md b/src/ComputeWeave.SourceGenerators/AnalyzerReleases.Unshipped.md index c78ea44d3..96ccbf94e 100644 --- a/src/ComputeWeave.SourceGenerators/AnalyzerReleases.Unshipped.md +++ b/src/ComputeWeave.SourceGenerators/AnalyzerReleases.Unshipped.md @@ -6,3 +6,4 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- CMPW0129 | ComputeWeave.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) +CMPW0130 | ComputeWeave.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) diff --git a/src/ComputeWeave.SourceGenerators/ComputeShaderDescriptorGenerator.HlslSource.cs b/src/ComputeWeave.SourceGenerators/ComputeShaderDescriptorGenerator.HlslSource.cs index c90001bc3..5e57d70a6 100644 --- a/src/ComputeWeave.SourceGenerators/ComputeShaderDescriptorGenerator.HlslSource.cs +++ b/src/ComputeWeave.SourceGenerators/ComputeShaderDescriptorGenerator.HlslSource.cs @@ -378,6 +378,7 @@ private static ImmutableArray GetStaticFields( CancellationToken token) { using ImmutableArrayBuilder declared = new(); + List declaredFields = []; foreach (ISymbol memberSymbol in structDeclarationSymbol.GetMembers()) { @@ -417,6 +418,7 @@ private static ImmutableArray GetStaticFields( typeDeclaration, assignmentExpression, HlslDefinitionsSyntaxProcessor.GetStaticFieldOrder(staticFieldDefinitions))); + declaredFields.Add(fieldSymbol); // An initializer may import a method that declares a local function, which HLSL cannot // nest, so the ones lifted out of it are carried up to be written like any other @@ -436,6 +438,19 @@ private static ImmutableArray GetStaticFields( diagnostics, token); + // A static constructor C# would run is not written into the generated HLSL, so a static field it + // assigns is reported, the walk covering the constructors of every type the rewriting touched + HlslDefinitionsSyntaxProcessor.ReportStaticFieldsAssignedByAStaticConstructor( + structDeclarationSymbol, + declaredFields, + staticMethods, + instanceMethods, + constructors, + staticFieldDefinitions, + semanticModel, + diagnostics, + token); + // The fields are written as one sequence, ordered by when each finished, which is after every // field its own initializer reached. A declared field and an imported one that finished with the // same count keep this order, the import having finished after the declared field did diff --git a/src/ComputeWeave.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/src/ComputeWeave.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index 0059ef051..fadc1a8d7 100644 --- a/src/ComputeWeave.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/src/ComputeWeave.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -1656,6 +1656,22 @@ partial class DiagnosticDescriptors description: "C# runs the static field initializers of a type once, in the order the fields are declared. A field whose turn has not come yet holds the default value of its type: a read of it is that default, and a write to it is discarded when its initializer runs. Both happen when an initializer reaches a field of the same type declared after it, directly or through any declaration it reaches, and the read happens when an initializer reaches back into the field it initializes. The generated HLSL reproduces neither: the shader compiler folds the initializer the field carries when it can, so the read is the initialized value and the write stays, leaves the read undefined otherwise, and does not compile a direct read of a global declared later at all, so the shader either computes a value C# never produces or fails against generated code the author never wrote. A field carrying no initializer holds the default value in both and is not reported, and neither is a write to the field being initialized, which its initializer overwrites in both. Before this report an initializer reaching itself faulted the generator instead, adding the same key to the collection of static field definitions twice, which discards the descriptors for every shader in the compilation unit and leaves the author with errors that name none of this.", helpLinkUri: "https://github.com/routersys/ComputeWeave"); + /// + /// Gets a for a static field assigned by a static constructor. + /// + /// Format: "The static field {0} is assigned by the static constructor of {1}, which the generated HLSL does not run (the field holds the value of its initializer there, or zero without one)". + /// + /// + public static readonly DiagnosticDescriptor StaticFieldAssignedByStaticConstructor = new( + id: "CMPW0130", + title: "Static field assigned by a static constructor", + messageFormat: "The static field {0} is assigned by the static constructor of {1}, which the generated HLSL does not run (the field holds the value of its initializer there, or zero without one)", + category: "ComputeWeave.Shaders", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "C# runs the static constructor of a type when the type is first touched, after the initializers of its static fields, and a static field the constructor assigns holds that value from then on. The generated HLSL runs no static constructor and declares the field with the value of its initializer, or with no initializer, which the shader compiler reads as zero, so the shader computes a value C# never produces and neither compiler reports anything. The static constructors of the shader type and of every type an imported declaration belongs to are followed through every declaration they reach, and a write to any static field the generated HLSL declares is reported, whichever of those types the constructor belongs to.", + helpLinkUri: "https://github.com/routersys/ComputeWeave"); + /// /// Gets a for an operation whose operands C# widens past the HLSL type set. /// diff --git a/src/ComputeWeave/README.ja.md b/src/ComputeWeave/README.ja.md index 4ad54a45b..0f4b3be21 100644 --- a/src/ComputeWeave/README.ja.md +++ b/src/ComputeWeave/README.ja.md @@ -94,7 +94,7 @@ public sealed partial class ResourceSet ## コンパイル時の検証 -以上の宣言はアナライザーが検査し、接頭辞 `CMPW` の診断113種類として報告します。対象は属性の位置、ホストとパイプラインメソッドの形、スロットの宣言、資源の契約、生成されるオーバーロードの衝突です。一部にはコード修正が付きます。実行時に識別子を持つ拒否は `IComputeDiagnostic` を実装するもの、すなわち `ComputeDiagnosticException` と `GraphicsDeviceMismatchException` だけです。その `DiagnosticId` は同じ `CMPW` 接頭辞を専用の番号帯で使います。引数の検査は識別子を持ちません。`ArgumentOutOfRangeException` をはじめとするフレームワークの例外型を送出し、代わりに引数の名前を載せます。 +以上の宣言はアナライザーが検査し、接頭辞 `CMPW` の診断114種類として報告します。対象は属性の位置、ホストとパイプラインメソッドの形、スロットの宣言、資源の契約、生成されるオーバーロードの衝突です。一部にはコード修正が付きます。実行時に識別子を持つ拒否は `IComputeDiagnostic` を実装するもの、すなわち `ComputeDiagnosticException` と `GraphicsDeviceMismatchException` だけです。その `DiagnosticId` は同じ `CMPW` 接頭辞を専用の番号帯で使います。引数の検査は識別子を持ちません。`ArgumentOutOfRangeException` をはじめとするフレームワークの例外型を送出し、代わりに引数の名前を載せます。 ## 詳細 diff --git a/src/ComputeWeave/README.md b/src/ComputeWeave/README.md index dac8b4397..93f8442af 100644 --- a/src/ComputeWeave/README.md +++ b/src/ComputeWeave/README.md @@ -94,7 +94,7 @@ Retiring a shared texture generation drains the external queue before the extern ## Compile-time validation -The declarations above are checked by analyzers that report 113 diagnostics with the `CMPW` prefix, covering attribute placement, host and pipeline method shape, slot declaration, resource contracts and generated overload conflicts. Some carry a code fix. At run time an identifier is carried only by the rejections that implement `IComputeDiagnostic`, which are `ComputeDiagnosticException` and `GraphicsDeviceMismatchException`; their `DiagnosticId` uses the same `CMPW` prefix in a number band of its own. Argument validation carries no identifier: it throws the framework exception types, `ArgumentOutOfRangeException` among them, and names the argument instead. +The declarations above are checked by analyzers that report 114 diagnostics with the `CMPW` prefix, covering attribute placement, host and pipeline method shape, slot declaration, resource contracts and generated overload conflicts. Some carry a code fix. At run time an identifier is carried only by the rejections that implement `IComputeDiagnostic`, which are `ComputeDiagnosticException` and `GraphicsDeviceMismatchException`; their `DiagnosticId` uses the same `CMPW` prefix in a number band of its own. Argument validation carries no identifier: it throws the framework exception types, `ArgumentOutOfRangeException` among them, and names the argument instead. ## More diff --git a/tests/ComputeWeave.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Diagnostics.cs b/tests/ComputeWeave.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Diagnostics.cs index c4701b76b..09d358714 100644 --- a/tests/ComputeWeave.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Diagnostics.cs +++ b/tests/ComputeWeave.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Diagnostics.cs @@ -1257,6 +1257,51 @@ public float4 Execute() CSharpGeneratorTest.VerifyDiagnosticIsReported(source, "CMPWD2D0096"); } + /// + /// A static field of another type that its static constructor assigns, read by the shader. C# runs the + /// constructor when the type is first touched, whereas the generated HLSL runs none and reads zero. + /// + /// + /// The walk is shared with the compute generator, and each of the two carries the descriptor under its own + /// identifier, so a row on one of them says nothing about the other. + /// + [TestMethod] + public void AStaticFieldAssignedByAStaticConstructorIsDiagnosed() + { + const string source = """ + using ComputeWeave; + using ComputeWeave.D2D1; + using float4 = global::ComputeWeave.Float4; + + namespace MyNamespace; + + internal static class Helper + { + public static readonly float Amount; + + static Helper() + { + Amount = 5.0f; + } + } + + [D2DInputCount(0)] + [D2DShaderProfile(D2D1ShaderProfile.PixelShader50)] + [D2DGeneratedPixelShaderDescriptor] + internal readonly partial struct MyShader : ID2D1PixelShader + { + private static readonly float Value = Helper.Amount; + + public float4 Execute() + { + return Value; + } + } + """; + + CSharpGeneratorTest.VerifyDiagnosticIsReported(source, "CMPWD2D0103"); + } + /// /// A static method of the shader reading a static field of it, reached from the body rather than from that /// field's initializer, which is the ordinary shape the walk answering the rows above runs over. diff --git a/tests/ComputeWeave.Tests.SourceGenerators/Shaders/ShaderGeneratorDiagnosticTests.cs b/tests/ComputeWeave.Tests.SourceGenerators/Shaders/ShaderGeneratorDiagnosticTests.cs index 37b60b4c5..a7a21117c 100644 --- a/tests/ComputeWeave.Tests.SourceGenerators/Shaders/ShaderGeneratorDiagnosticTests.cs +++ b/tests/ComputeWeave.Tests.SourceGenerators/Shaders/ShaderGeneratorDiagnosticTests.cs @@ -1073,6 +1073,194 @@ public void AStaticFieldAccessIsAttributedToTheInitializerPerformingIt() Assert.AreEqual("Second in First, Third in Second", string.Join(", ", attributions)); } + /// + /// A static field a static constructor assigns: a field of another type, with or without an initializer of + /// its own, assigned directly, through a method the constructor calls or through an out argument; a field + /// of the shader assigned by the shader's own static constructor; and a field of the shader assigned by the + /// static constructor of a type the body imports a method from. C# runs the constructor when its type is + /// first touched, whereas the generated HLSL runs none and reads the initializer's value, or zero without one. + /// + [TestMethod] + [DataRow( + "ExternalStaticFieldAssignedByAStaticConstructorTests", + """ + internal static class Helper + { + public static readonly float Amount; + + static Helper() + { + Amount = 5.0f; + } + } + """, + "internal static readonly float Value = Helper.Amount;", + "this.buffer[0] = Value;")] + [DataRow( + "ExternalMutableStaticFieldAssignedByAStaticConstructorTests", + """ + internal static class Helper + { + public static float Amount; + + static Helper() + { + Amount = 5.0f; + } + } + """, + "internal static readonly float Value = Helper.Amount;", + "this.buffer[0] = Value;")] + [DataRow( + "ExternalInitializedStaticFieldOverwrittenByAStaticConstructorTests", + """ + internal static class Helper + { + public static readonly float Amount = 2.0f; + + static Helper() + { + Amount = 5.0f; + } + } + """, + "internal static readonly float Value = Helper.Amount;", + "this.buffer[0] = Value;")] + [DataRow( + "ExternalStaticFieldAssignedByAMethodAStaticConstructorCallsTests", + """ + internal static class Helper + { + public static float Amount; + + static Helper() + { + Fill(); + } + + private static void Fill() + { + Amount = 5.0f; + } + } + """, + "internal static readonly float Value = Helper.Amount;", + "this.buffer[0] = Value;")] + [DataRow( + "ExternalStaticFieldAssignedThroughAnOutArgumentInAStaticConstructorTests", + """ + internal static class Helper + { + public static readonly float Amount; + + static Helper() + { + Fill(out Amount); + } + + private static void Fill(out float target) + { + target = 5.0f; + } + } + """, + "internal static readonly float Value = Helper.Amount;", + "this.buffer[0] = Value;")] + [DataRow( + "ShaderStaticFieldAssignedByTheShaderStaticConstructorTests", + "", + """ + private static readonly float Value; + + static Shader() + { + Value = 5.0f; + } + """, + "this.buffer[0] = Value;")] + [DataRow( + "ShaderStaticFieldAssignedByTheStaticConstructorOfAnImportedMethodTypeTests", + """ + internal static class Helper + { + static Helper() + { + Shader.Value = 5.0f; + } + + public static float Go() => 2.0f; + } + """, + "internal static float Value = 1.0f;", + "this.buffer[0] = Helper.Go() + Value;")] + public void AStaticFieldAssignedByAStaticConstructorIsDiagnosed(string assemblyName, string declarations, string members, string body) + { + AssertReportsAt(ShaderWithStaticFields(members, body, declarations), assemblyName, "CMPW0130", 1); + } + + /// + /// The static constructors that leave the fields the generated HLSL declares alone: one reading the field, + /// one assigning a field of its type the shader does not use, and one of a type the shader imports a method + /// from that assigns nothing the shader uses. + /// + /// + /// The set is pinned empty rather than the identifier counted, so the rows also show the shader compiles. + /// + [TestMethod] + [DataRow( + "ExternalStaticFieldReadByAStaticConstructorTests", + """ + internal static class Helper + { + public static readonly float Amount = 2.0f; + + static Helper() + { + _ = Amount; + } + } + """, + "internal static readonly float Value = Helper.Amount;", + "this.buffer[0] = Value;")] + [DataRow( + "ExternalStaticFieldBesideOneAStaticConstructorAssignsTests", + """ + internal static class Helper + { + public static readonly float Amount = 2.0f; + + public static float Other; + + static Helper() + { + Other = 5.0f; + } + } + """, + "internal static readonly float Value = Helper.Amount;", + "this.buffer[0] = Value;")] + [DataRow( + "ImportedMethodTypeWithAStaticConstructorAssigningNothingTheShaderUsesTests", + """ + internal static class Helper + { + public static float Other; + + static Helper() + { + Other = 5.0f; + } + + public static float Go() => 2.0f; + } + """, + "internal static readonly float Value = 1.0f;", + "this.buffer[0] = Helper.Go() + Value;")] + public void AStaticConstructorLeavingTheDeclaredStaticFieldsAloneIsNotDiagnosed(string assemblyName, string declarations, string members, string body) + { + AssertReportsNothing(ShaderWithStaticFields(members, body, declarations), assemblyName); + } + private static string ShaderWithStaticFields(string members, string body = "this.buffer[0] = Value;", string declarations = "") { return $$"""