diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aa68d3ad..6714b23d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -672,6 +672,7 @@ The Direct2D authoring projects were removed before the first release and restor | Generated HLSL, static field initializers | Rewrites a static field initializer with a rewriter that only maps intrinsics. A call to a static method declared outside the shader is written out as it stands, naming a type the generated HLSL never declares, and the shader compiler rejects it while naming generated code the author never wrote. The same call one line away in the shader body is imported, so where it is written decides whether it works | Imports the declaration the same way the body does, and renames the call to it. The forward declarations are written ahead of the static fields, so a call to an imported function is in scope where the initializer needs it, which was measured on both the DXC and the FXC path before the import was written. The local functions lifted out of an imported method are carried out to the caller and written like any other. The static methods are gathered after the static fields, an initializer being able to import one of its own | `70550094` | | Generated HLSL, a constructor in a static field initializer | Hands a user defined constructor call to an extension point whose default writes a default value, and overrides that default only in the rewriter for the shader body. An initializer holds zero where the same call one line away in the body constructs the value, and nothing is reported on either path: the generated HLSL still compiles, so the only sign is the number the shader computes | Imports the constructor the way the body does, the initializer rewriter handing the call to the one that rewrites bodies, a constructor declaration being a body. The arguments keep the rewriting the initializer gave them, the import reading them from the node it is handed rather than visiting them, and the local functions it lifts out are carried over the way an imported method's already are. The stubs are written among the type declarations, which both generators write ahead of the static fields, so the call is in scope where the initializer needs it, measured on the DXC path and on the FXC path alike; the value the field holds is read back from a device rather than inferred from the two paths writing the same source. The parameterless constructor a struct always has stays a default value, which is what it computes in C# as well. The extension point carries no default any more, one answering with a default value having computed something other than what the author wrote without saying so, so a rewriter that will not import a constructor states what it does instead | `b3bd8a3a`, `40fe41df`, `03433ef5`, `3220e6f4` | | Generated HLSL, a constructor written with an expression body | Reads the body of a constructor declaration without first turning an expression body into a block, which both of the other declarations it visits do, so importing one ends the generator with a null reference. The output of that generator is then dropped for the whole compilation unit, and what the author reads is a list of unrelated errors naming generated types that were never written. The two forms mean the same thing in C#, and the accepted set holds the arrow clause and the constructor declaration alike, so nothing had judged the shape refused | Turns the body into a block the way a method and a local function already are, from an override of the visit rather than from the caller, so that a route reaching a constructor declaration later is normalized as well. Only the body is: the import builds the declarations it writes from the parameter list and the body alone, and reads neither the modifiers nor the attribute lists, which is why a method drops its modifiers and a constructor drops nothing, an attribute list being dropped for every declaration by the walk the two share. Both entry points, the shader body and a static field initializer, were measured failing at that one line before the change and writing the same HLSL as a block body after it, on the Direct2D path as much as on the compute one. The generated sources of the whole solution were compared file by file across the change, and the only two that move are the pair that moves between two builds of the same commit anyway | `b576c5dc`, `207e40fc` | +| Generated HLSL, a declaration carrying no body | Reads the body of a declaration without asking whether it has one. An extern declaration carries none, which C# reports only as a warning, and every route that reads a declaration meets it. A method and a local function are written out as declarations with no body and the shader compiler answers by naming generated code the author never wrote; a constructor and the entry point end the generator instead, which discards the descriptors for every shader in the compilation unit. Eleven routes were measured, which are the ones that write a declaration into the generated HLSL: the four imports, the entry point of each of the two generators, and a method of the shader itself or a local function, the last two whether or not they are called | Reports `CMPW0127`, or `CMPWD2D0099` on the Direct2D path, from the visit that normalizes the body of each of the three kinds a declaration can be, so the four imports and the entry point of each of the two generators are answered by one rule rather than by one rule per route. The declaration is given an empty body rather than the rewriting ending there: ending it discards the output for the whole compilation unit, which is the failure the report replaces, and the report is an error, so what the empty body produces never reaches the shader compiler. The helper that normalizes a body already stated it always returns a block body, and it now keeps that for a declaration carrying none as well. A declaration split into parts is unaffected, the implementing part being the one that is read. What is reported follows what is written out rather than what is declared, so a member of an external type the shader never reaches is left alone, which three routes were measured for. A partial declaration with no implementing part is outside this: C# refuses one carrying accessibility modifiers and erases one that does not, so neither reaches the rewriting | `a86382b6`, `56f951ff` | | Generated HLSL, a static field initializer reaching itself | Rewrites the initializer of an external static field and adds the entry to the collection of static field definitions after the rewriting finishes, where the two other collections a rewriting can return to claim their entry before it. An initializer that reaches the field it initializes therefore finds no entry, rewrites the field a second time and adds it, and the outer rewriting adds the same key again when it returns. The generator faults with an argument exception, which discards the descriptors for every shader in the compilation unit and leaves the author with errors that name none of this. The two routes that close the cycle are a method and a constructor the initializer imports | Claims the entry before rewriting, the way the other two collections do, and treats a return to a claimed entry as a cycle, reporting `CMPW0124` on the compute path and `CMPWD2D0096` on the Direct2D one. Claiming alone was measured to be insufficient: the fault stops, no diagnostic is reported, and the shader compiler accepts generated HLSL that reads a global static through a function reading it back, so the fault would become a silently different value, which is the worse of the two in the order this tree keeps. The mark is an empty type declaration, which widens the type alias for a static field; the two places that read it both read an array built after every rewriting finishes, and building both generators over the widened alias reports no warning. One signature wrote the collection as a raw tuple rather than as the alias, which the widening turns into a nullability error, so it is aligned with the other three. The report is located on the read that closes the cycle and not on the field declaration: a cycle closed from two imported declarations is two places the author has to change, and one report on the declaration names neither of them. A static field of the shader is excluded by every call site here too, and is answered on routes of its own, which a row of its own records. An initializer reading a static field directly reaches the report as well, that read being imported the way the shader body imports one. | `da160eb1`, `fc0676b9`, `a1a12303`, `fabf4202` | | Generated HLSL, static field reads in an initializer | Rewrites a static field initializer with a rewriter that answers for a constant and for nothing else a field can be. A read of a static field declared outside the shader is written out under the name the author wrote, naming a type the generated HLSL never declares, and a read of the shader's own static field through the type name keeps the type name on it. The shader body imports the first and drops the type name from the second, one line away. The initializer of an imported field is rewritten by the same rewriter, so the body meets this as well as soon as an imported field's own initializer reads another one by name alone. Nothing is reported on any of these paths, the shader compiler answering instead by naming generated code the author never wrote. The fields the body does import are written after the ones the shader type declares, and among themselves in whatever order the collection holding them enumerates, so a read that resolved to an import would still name a declaration written after it | Reads a static field the way the body does, handing the read to the rewriter that imports every other declaration an initializer reaches, the way the import of a constructor is already handed to it, and carrying back the local functions that import lifted out. A field of the shader type is written under the name it has in the shader rather than imported, which is what the body does with it. A read written by name alone is intercepted before the member access path, an identifier being where a read of a field of the enclosing type arrives, and a read the import declines falls back to what the base rewriter would have done, which keeps a constant and an enum member on the path they already had. Every static field, imported or declared on the shader type, is written as one sequence ordered by how many had finished when its own rewriting did, which is after every field its initializer reached, so each is written after the ones it names; the count is carried on the model for a static field rather than left to the enumeration order of the collection. Writing the imported ones as a block ahead of the declared ones was measured to be insufficient, a field declared on the shader type and read from an imported initializer landing after the read. The import claims its entry before rewriting, so an initializer reaching the field it initializes through one of these reads is answered by `CMPW0124`, or `CMPWD2D0096` on the Direct2D path, rather than by a fault | `3bc1bcca`, `19fe5d51` | | 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` | diff --git a/README.ja.md b/README.ja.md index c5e476b1..275315a4 100644 --- a/README.ja.md +++ b/README.ja.md @@ -259,7 +259,7 @@ GPUが書いたバッファを、以降のシェーダーへ読み取り専用 ### コンパイル時の検証 -以上の宣言はアナライザーが検査し、接頭辞 `CMPW` の診断110種類として報告します。対象は属性の位置、ホストとパイプラインメソッドの形、スロットの宣言、資源の契約、生成されるオーバーロードの衝突です。一部にはコード修正が付きます。 +以上の宣言はアナライザーが検査し、接頭辞 `CMPW` の診断111種類として報告します。対象は属性の位置、ホストとパイプラインメソッドの形、スロットの宣言、資源の契約、生成されるオーバーロードの衝突です。一部にはコード修正が付きます。 --- @@ -296,7 +296,7 @@ ReadOnlyMemory bytecode = D2D1PixelShader.LoadBytecode() ReadOnlyMemory buffer = D2D1PixelShader.GetConstantBuffer(new DifferenceEffect(1)); ``` -これらの宣言は、`CMPWD2D` を接頭辞とする98件の診断を報告するアナライザーが検査します。シェーダーは Direct2D が受け付ける DXBC へ FXC でコンパイルします。`d3dcompiler_47.dll` は Windows に同梱されているため、このパッケージはコンパイラーを同梱しません。 +これらの宣言は、`CMPWD2D` を接頭辞とする99件の診断を報告するアナライザーが検査します。シェーダーは Direct2D が受け付ける DXBC へ FXC でコンパイルします。`d3dcompiler_47.dll` は Windows に同梱されているため、このパッケージはコンパイラーを同梱しません。 --- diff --git a/README.md b/README.md index b5907473..e0b73f23 100644 --- a/README.md +++ b/README.md @@ -259,7 +259,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 110 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 111 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. --- @@ -296,7 +296,7 @@ ReadOnlyMemory bytecode = D2D1PixelShader.LoadBytecode() ReadOnlyMemory buffer = D2D1PixelShader.GetConstantBuffer(new DifferenceEffect(1)); ``` -The declarations are checked by analyzers that report 98 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 99 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 bf38dbd9..5ab98e09 100644 --- a/src/ComputeWeave.D2D1.SourceGenerators/AnalyzerReleases.Unshipped.md +++ b/src/ComputeWeave.D2D1.SourceGenerators/AnalyzerReleases.Unshipped.md @@ -9,6 +9,7 @@ CMPWD2D0095 | ComputeWeave.D2D1.Shaders | Error | [Documentation](https://github CMPWD2D0096 | ComputeWeave.D2D1.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) CMPWD2D0097 | ComputeWeave.D2D1.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) CMPWD2D0098 | ComputeWeave.D2D1.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) +CMPWD2D0099 | ComputeWeave.D2D1.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) ; The rule below changed severity rather than being added, so it moves into the shipped file under a ; "Changed Rules" heading of its own. build/verify-analyzer-releases.ps1 refuses it under "New Rules". diff --git a/src/ComputeWeave.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/src/ComputeWeave.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index 8450df93..83353815 100644 --- a/src/ComputeWeave.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/src/ComputeWeave.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -1497,4 +1497,24 @@ partial class DiagnosticDescriptors isEnabledByDefault: true, description: "An out argument written as a declaration and a discarded one both need a variable the rewriting introduces, which a shader body declares at the start of the body and passes to the call. A static field initializer is written as one HLSL expression, so there is nowhere ahead of it to put that declaration. Giving the variable a global static of its own instead would share one storage across every invocation of the shader, and HLSL leaves the order of global static initializers undefined, so the field could read a value C# never computes. Without this the declaration is written into the call as it stands, and the shader compiler answers by naming generated code the author never wrote.", helpLinkUri: "https://github.com/routersys/ComputeWeave"); + + /// + /// Gets a for a declaration that carries no body. + /// + /// Format: "The declaration of {0} cannot be used in a D2D1 pixel shader (it carries no body, so there is nothing to write into the generated HLSL)". + /// + /// + /// + /// This is the Direct2D counterpart of the compute diagnostic. The rewriter that reports it is shared, so + /// leaving one of the two out would refuse the same declaration on one path and not the other. + /// + public static readonly DiagnosticDescriptor DeclarationWithNoBody = new( + id: "CMPWD2D0099", + title: "Declaration with no body", + messageFormat: "The declaration of {0} cannot be used in a D2D1 pixel shader (it carries no body, so there is nothing to write into the generated HLSL)", + category: "ComputeWeave.D2D1.Shaders", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "What is written into the generated HLSL is built from the body of the declaration, so one that carries none has nothing to write. An extern declaration is that case, and C# reports only a warning for it. Without this a method or a local function is written out as a declaration with no body and the shader compiler answers by naming generated code the author never wrote, while a constructor and the entry point end the generator instead, which discards the descriptors for every shader in the compilation unit. What is reported follows what is written out: a member of an external type is written out where the shader reaches it, and one it never reaches is left alone. A declaration split into parts is unaffected, the implementing part being the one that is read.", + helpLinkUri: "https://github.com/routersys/ComputeWeave"); } \ No newline at end of file diff --git a/src/ComputeWeave.SourceGeneration.Hlsl/Extensions/SyntaxNodeExtensions.cs b/src/ComputeWeave.SourceGeneration.Hlsl/Extensions/SyntaxNodeExtensions.cs index aa7b1bd3..9c778711 100644 --- a/src/ComputeWeave.SourceGeneration.Hlsl/Extensions/SyntaxNodeExtensions.cs +++ b/src/ComputeWeave.SourceGeneration.Hlsl/Extensions/SyntaxNodeExtensions.cs @@ -30,6 +30,14 @@ PredefinedTypeSyntax pts when pts.Keyword.IsKind(SyntaxKind.VoidKeyword) => Expr .WithSemicolonToken(MissingToken(SyntaxKind.SemicolonToken)); } + // The promise is a block body, so a declaration carrying none is given an empty one + if (node.Body is null) + { + return node + .WithBody(Block()) + .WithSemicolonToken(MissingToken(SyntaxKind.SemicolonToken)); + } + return node; } @@ -88,6 +96,14 @@ PredefinedTypeSyntax pts when pts.Keyword.IsKind(SyntaxKind.VoidKeyword) => Expr .WithSemicolonToken(MissingToken(SyntaxKind.SemicolonToken)); } + // The promise is a block body, so a declaration carrying none is given an empty one + if (node.Body is null) + { + return node + .WithBody(Block()) + .WithSemicolonToken(MissingToken(SyntaxKind.SemicolonToken)); + } + return node; } @@ -138,6 +154,14 @@ public static ConstructorDeclarationSyntax WithBlockBody(this ConstructorDeclara .WithSemicolonToken(MissingToken(SyntaxKind.SemicolonToken)); } + // The promise is a block body, so a declaration carrying none is given an empty one + if (node.Body is null) + { + return node + .WithBody(Block()) + .WithSemicolonToken(MissingToken(SyntaxKind.SemicolonToken)); + } + return node; } diff --git a/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxRewriters/HlslSourceRewriter.Diagnostics.cs b/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxRewriters/HlslSourceRewriter.Diagnostics.cs index 8541cc9e..f6a2a1c9 100644 --- a/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxRewriters/HlslSourceRewriter.Diagnostics.cs +++ b/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxRewriters/HlslSourceRewriter.Diagnostics.cs @@ -51,6 +51,41 @@ protected void ReportSyntaxOutsideTheAcceptedSet(SyntaxNode node) } } + /// + /// Reports a declaration carrying no body, there being nothing to write out for one. + /// + /// The declaration a lookup handed to the rewriting. + /// The identifier naming . + /// The block body of , if it has one. + /// The expression body of , if it has one. + /// + /// + /// What is written into the generated HLSL is built from the body, so a declaration carrying none has + /// nothing to write. This is called from the visit that normalizes the body of each of the three kinds a + /// declaration can be, which is where every declaration that is written out arrives: the four imports and + /// the entry point of each of the two generators, and a method of the shader itself or a local function + /// as well, neither of which is looked up and both of which are written out whether or not they are + /// called. A member of an external type the shader never reaches is not written out and is not reported. + /// + /// + /// The rewriting continues over an empty body rather than ending here. Ending it discards the output for + /// the whole compilation unit, which leaves the author with errors naming generated types instead of the + /// declaration to change, and that is the failure this replaces. The report is an error, so what the + /// empty body produces never reaches the shader compiler. + /// + /// + protected void ReportDeclarationWithNoBody( + SyntaxNode node, + SyntaxToken identifier, + BlockSyntax? body, + ArrowExpressionClauseSyntax? expressionBody) + { + if (body is null && expressionBody is null) + { + Diagnostics.Add(DeclarationWithNoBody, node, identifier.Text); + } + } + /// /// Reports a member access that every mapping has declined, when HLSL cannot express it. /// diff --git a/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxRewriters/ShaderSourceRewriter.cs b/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxRewriters/ShaderSourceRewriter.cs index 16d783bb..fd9d6e59 100644 --- a/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxRewriters/ShaderSourceRewriter.cs +++ b/src/ComputeWeave.SourceGeneration.Hlsl/SyntaxRewriters/ShaderSourceRewriter.cs @@ -296,12 +296,16 @@ public override SyntaxNode VisitLocalDeclarationStatement(LocalDeclarationStatem /// public override SyntaxNode? VisitConstructorDeclaration(ConstructorDeclarationSyntax node) { + ReportDeclarationWithNoBody(node, node.Identifier, node.Body, node.ExpressionBody); + return ((ConstructorDeclarationSyntax)base.VisitConstructorDeclaration(node)!).WithBlockBody(); } /// public override SyntaxNode? VisitMethodDeclaration(MethodDeclarationSyntax node) { + ReportDeclarationWithNoBody(node, node.Identifier, node.Body, node.ExpressionBody); + return ((MethodDeclarationSyntax)base.VisitMethodDeclaration(node)!) .WithBlockBody() @@ -313,6 +317,8 @@ public override SyntaxNode VisitLocalDeclarationStatement(LocalDeclarationStatem { CancellationToken.ThrowIfCancellationRequested(); + ReportDeclarationWithNoBody(node, node.Identifier, node.Body, node.ExpressionBody); + // If the current identifier matches the one for the current method, it means the local function // statement is the one being inspected, and it's nto coming from a local function in a method // being parsed. That is, this was a blobal method that was annotated in source. diff --git a/src/ComputeWeave.SourceGenerators/AnalyzerReleases.Unshipped.md b/src/ComputeWeave.SourceGenerators/AnalyzerReleases.Unshipped.md index 19ef5da0..ccfb05d3 100644 --- a/src/ComputeWeave.SourceGenerators/AnalyzerReleases.Unshipped.md +++ b/src/ComputeWeave.SourceGenerators/AnalyzerReleases.Unshipped.md @@ -10,6 +10,7 @@ CMPW0123 | ComputeWeave.Shaders | Error | [Documentation](https://github.com/rou CMPW0124 | ComputeWeave.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) CMPW0125 | ComputeWeave.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) CMPW0126 | ComputeWeave.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) +CMPW0127 | ComputeWeave.Shaders | Error | [Documentation](https://github.com/routersys/ComputeWeave) ; The rule below changed severity rather than being added, so it moves into the shipped file under a ; "Changed Rules" heading of its own. build/verify-analyzer-releases.ps1 refuses it under "New Rules". diff --git a/src/ComputeWeave.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/src/ComputeWeave.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index 06acc233..9078cbfc 100644 --- a/src/ComputeWeave.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/src/ComputeWeave.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -1671,4 +1671,20 @@ partial class DiagnosticDescriptors isEnabledByDefault: true, description: "An out argument written as a declaration and a discarded one both need a variable the rewriting introduces, which a shader body declares at the start of the body and passes to the call. A static field initializer is written as one HLSL expression, so there is nowhere ahead of it to put that declaration. Giving the variable a global static of its own instead would share one storage across every invocation of the shader, and HLSL leaves the order of global static initializers undefined, so the field could read a value C# never computes. Without this the declaration is written into the call as it stands, and the shader compiler answers by naming generated code the author never wrote.", helpLinkUri: "https://github.com/routersys/ComputeWeave"); + + /// + /// Gets a for a declaration that carries no body. + /// + /// Format: "The declaration of {0} cannot be used in a compute shader (it carries no body, so there is nothing to write into the generated HLSL)". + /// + /// + public static readonly DiagnosticDescriptor DeclarationWithNoBody = new( + id: "CMPW0127", + title: "Declaration with no body", + messageFormat: "The declaration of {0} cannot be used in a compute shader (it carries no body, so there is nothing to write into the generated HLSL)", + category: "ComputeWeave.Shaders", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "What is written into the generated HLSL is built from the body of the declaration, so one that carries none has nothing to write. An extern declaration is that case, and C# reports only a warning for it. Without this a method or a local function is written out as a declaration with no body and the shader compiler answers by naming generated code the author never wrote, while a constructor and the entry point end the generator instead, which discards the descriptors for every shader in the compilation unit. What is reported follows what is written out: a member of an external type is written out where the shader reaches it, and one it never reaches is left alone. A declaration split into parts is unaffected, the implementing part being the one that is read.", + helpLinkUri: "https://github.com/routersys/ComputeWeave"); } \ No newline at end of file diff --git a/src/ComputeWeave/README.ja.md b/src/ComputeWeave/README.ja.md index b7edc543..3c587d66 100644 --- a/src/ComputeWeave/README.ja.md +++ b/src/ComputeWeave/README.ja.md @@ -94,7 +94,7 @@ public sealed partial class ResourceSet ## コンパイル時の検証 -以上の宣言はアナライザーが検査し、接頭辞 `CMPW` の診断110種類として報告します。実行時の拒否も同じ `CMPW` 接頭辞を使い、番号帯で区別します。対象は属性の位置、ホストとパイプラインメソッドの形、スロットの宣言、資源の契約、生成されるオーバーロードの衝突です。一部にはコード修正が付きます。 +以上の宣言はアナライザーが検査し、接頭辞 `CMPW` の診断111種類として報告します。実行時の拒否も同じ `CMPW` 接頭辞を使い、番号帯で区別します。対象は属性の位置、ホストとパイプラインメソッドの形、スロットの宣言、資源の契約、生成されるオーバーロードの衝突です。一部にはコード修正が付きます。 ## 詳細 diff --git a/src/ComputeWeave/README.md b/src/ComputeWeave/README.md index 00f29adb..7804a60d 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 110 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. Runtime rejections use the same `CMPW` prefix, told apart by their number band. +The declarations above are checked by analyzers that report 111 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. Runtime rejections use the same `CMPW` prefix, told apart by their number band. ## More diff --git a/tests/ComputeWeave.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Diagnostics.cs b/tests/ComputeWeave.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Diagnostics.cs index b4dcdf7c..edca6c11 100644 --- a/tests/ComputeWeave.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Diagnostics.cs +++ b/tests/ComputeWeave.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Diagnostics.cs @@ -1022,4 +1022,111 @@ public float4 Execute() CSharpGeneratorTest.VerifyDiagnostics(source); } + + /// + /// A declaration carrying no body, imported by the shader body. What reaches the generated HLSL is built + /// from the body, so one that has none has nothing to write, and C# reports an extern declaration as a + /// warning rather than an error. + /// + /// + /// The rewriting that reports this is shared with the compute generator, and each of the two carries its + /// own identifier, so a row on one of them says nothing about the other. + /// + [TestMethod] + public void ADeclarationWithNoBodyIsDiagnosed() + { + const string source = """ + using ComputeWeave; + using ComputeWeave.D2D1; + using float4 = global::ComputeWeave.Float4; + + namespace MyNamespace; + + internal static class Helper + { + public static extern float Twice(float value); + } + + [D2DInputCount(0)] + [D2DShaderProfile(D2D1ShaderProfile.PixelShader50)] + [D2DGeneratedPixelShaderDescriptor] + internal readonly partial struct MyShader : ID2D1PixelShader + { + private readonly float time; + + public float4 Execute() + { + return Helper.Twice(this.time); + } + } + """; + + CSharpGeneratorTest.VerifyDiagnostics(source, "CMPWD2D0099"); + } + + /// + /// A constructor carrying no body, imported by the shader body. This is the route that ends the generator + /// without the report, so the row answers for the rewriting finishing as well as for the identifier. + /// + [TestMethod] + public void AConstructorWithNoBodyIsDiagnosed() + { + const string source = """ + using ComputeWeave; + using ComputeWeave.D2D1; + using float4 = global::ComputeWeave.Float4; + + namespace MyNamespace; + + internal struct Helper + { + public float Amount; + + public extern Helper(float amount); + } + + [D2DInputCount(0)] + [D2DShaderProfile(D2D1ShaderProfile.PixelShader50)] + [D2DGeneratedPixelShaderDescriptor] + internal readonly partial struct MyShader : ID2D1PixelShader + { + private readonly float time; + + public float4 Execute() + { + return new Helper(this.time).Amount; + } + } + """; + + CSharpGeneratorTest.VerifyDiagnostics(source, "CMPWD2D0099"); + } + + /// + /// The entry point written with no body, which is the one route that imports no declaration and the one + /// this generator reads for itself. + /// + [TestMethod] + public void AnEntryPointWithNoBodyIsDiagnosed() + { + const string source = """ + using ComputeWeave; + using ComputeWeave.D2D1; + using float4 = global::ComputeWeave.Float4; + + namespace MyNamespace; + + [D2DInputCount(0)] + [D2DShaderProfile(D2D1ShaderProfile.PixelShader50)] + [D2DGeneratedPixelShaderDescriptor] + internal readonly partial struct MyShader : ID2D1PixelShader + { + private readonly float time; + + public extern float4 Execute(); + } + """; + + CSharpGeneratorTest.VerifyDiagnostics(source, "CMPWD2D0099"); + } } diff --git a/tests/ComputeWeave.Tests.SourceGenerators/Shaders/DeclarationWithNoBodyTests.cs b/tests/ComputeWeave.Tests.SourceGenerators/Shaders/DeclarationWithNoBodyTests.cs new file mode 100644 index 00000000..2825489b --- /dev/null +++ b/tests/ComputeWeave.Tests.SourceGenerators/Shaders/DeclarationWithNoBodyTests.cs @@ -0,0 +1,325 @@ +using System.Collections.Immutable; +using System.Linq; +using ComputeWeave.SourceGenerators; +using ComputeWeave.Tests.SourceGenerators.Helpers; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace ComputeWeave.Tests.SourceGenerators.Shaders; + +/// +/// What a declaration carrying no body is answered with. What reaches the generated HLSL is built from the +/// body, so a declaration that has none has nothing to write, and C# allows one: it reports an extern +/// declaration as a warning. +/// +/// +/// The rows are the routes a declaration reaches the rewriting by rather than the shapes an extern +/// declaration can take, the rewriting being what has to answer. Every route lands on the visit that +/// normalizes the body of one of the three kinds a declaration can be, so those three cover the four imports +/// and the entry point of each of the two generators. +/// +[TestClass] +public class DeclarationWithNoBodyTests +{ + [TestMethod] + [DataRow( + "BodilessStaticMethodFromBodyTests", + """ + internal static class Helper + { + public static extern float Twice(float value); + } + """, + "", + "this.buffer[0] = Helper.Twice(2.0f);")] + [DataRow( + "BodilessInstanceMethodFromBodyTests", + """ + internal struct Helper + { + public float Amount; + + public extern float Doubled(); + } + """, + "", + """ + Helper helper = default; + + this.buffer[0] = helper.Doubled(); + """)] + [DataRow( + "BodilessStaticMethodFromInitializerTests", + """ + internal static class Helper + { + public static extern float Twice(float value); + } + """, + "private static readonly float Scale = Helper.Twice(2.0f);", + "this.buffer[0] = Scale;")] + [DataRow( + "BodilessConstructorFromBodyTests", + """ + internal struct Helper + { + public float Amount; + + public extern Helper(float amount); + } + """, + "", + "this.buffer[0] = new Helper(2.0f).Amount;")] + [DataRow( + "BodilessConstructorFromInitializerTests", + """ + internal struct Helper + { + public float Amount; + + public extern Helper(float amount); + + public static float Read(Helper helper) => helper.Amount; + } + """, + "private static readonly float Scale = Helper.Read(new Helper(2.0f));", + "this.buffer[0] = Scale;")] + [DataRow( + "BodilessMethodOfTheShaderTests", + "", + "private extern float Twice(float value);", + "this.buffer[0] = Twice(2.0f);")] + [DataRow( + "BodilessLocalFunctionInTheBodyTests", + "", + "", + """ + static extern float Twice(float value); + + this.buffer[0] = Twice(2.0f); + """)] + [DataRow( + "BodilessLocalFunctionInAnImportTests", + """ + internal static class Helper + { + public static float Outer(float value) + { + static extern float Twice(float inner); + + return Twice(value); + } + } + """, + "", + "this.buffer[0] = Helper.Outer(2.0f);")] + [DataRow( + "BodilessUncalledMethodOfTheShaderTests", + "", + "private extern float Unused(float value);", + "this.buffer[0] = 1.0f;")] + [DataRow( + "BodilessUncalledLocalFunctionTests", + "", + "", + """ + static extern float Unused(float value); + + this.buffer[0] = 1.0f; + """)] + public void ADeclarationWithNoBodyIsDiagnosed(string assemblyName, string declarations, string members, string body) + { + AssertReportsOnly(Shader(declarations, members, body), assemblyName, "CMPW0127"); + } + + /// + /// The entry point written with no body, which is the one route that imports no declaration. + /// + [TestMethod] + public void AnEntryPointWithNoBodyIsDiagnosed() + { + const string source = """ + using ComputeWeave; + + namespace Shaders; + + [ThreadGroupSize(DefaultThreadGroupSizes.X)] + [GeneratedComputeShaderDescriptor] + internal readonly partial struct Shader : IComputeShader + { + private readonly ReadWriteBuffer buffer; + + public extern void Execute(); + } + """; + + AssertReportsOnly(source, "BodilessEntryPointTests", "CMPW0127"); + } + + /// + /// The same three kinds written with a body, so that the rows above answer for the body being absent + /// rather than for the kind. Each is required to produce source as well, an identifier being absent from + /// a run that generated nothing saying nothing. + /// + [TestMethod] + [DataRow( + "MethodWithABodyTests", + """ + internal static class Helper + { + public static float Twice(float value) => value * 2; + } + """, + "", + "this.buffer[0] = Helper.Twice(2.0f);")] + [DataRow( + "ConstructorWithABodyTests", + """ + internal struct Helper + { + public float Amount; + + public Helper(float amount) + { + Amount = amount; + } + } + """, + "", + "this.buffer[0] = new Helper(2.0f).Amount;")] + [DataRow( + "LocalFunctionWithABodyTests", + "", + "", + """ + static float Twice(float value) => value * 2; + + this.buffer[0] = Twice(2.0f); + """)] + public void ADeclarationWithABodyIsNotDiagnosed(string assemblyName, string declarations, string members, string body) + { + GeneratorRunResult result = Run(Shader(declarations, members, body), assemblyName); + + Assert.IsTrue(result.Diagnostics.IsEmpty, string.Join(", ", result.Diagnostics.Select(static diagnostic => diagnostic.Id))); + Assert.AreNotEqual(0, GeneratorHelper.GetGeneratedSource(result.GeneratedSources, "Shaders.Shader").Length); + } + + /// + /// A declaration carrying no body that the generated HLSL never holds. What is reported follows what is + /// written out rather than what is declared: a member of an external type is written out when the shader + /// reaches it, so one the shader never reaches is left alone the way it is today. + /// + /// + /// The rows above cover the other side of the same rule. A method of the shader itself and a local + /// function are written out whether or not they are called, so both are reported there. + /// + [TestMethod] + [DataRow( + "UncalledBodilessMethodOnAStructTests", + """ + internal struct Helper + { + public float Amount; + + public extern float Unused(); + + public float Doubled() + { + return Amount * 2; + } + } + """, + "", + """ + Helper helper = default; + + this.buffer[0] = helper.Doubled(); + """)] + [DataRow( + "UncalledBodilessMethodOnAStaticClassTests", + """ + internal static class Helper + { + public static extern float Unused(float value); + + public static float Twice(float value) => value * 2; + } + """, + "", + "this.buffer[0] = Helper.Twice(2.0f);")] + [DataRow( + "UncalledBodilessConstructorTests", + """ + internal struct Helper + { + public float Amount; + + public extern Helper(float amount, float unused); + + public Helper(float amount) + { + Amount = amount; + } + } + """, + "", + "this.buffer[0] = new Helper(2.0f).Amount;")] + public void ADeclarationTheGeneratedHlslNeverHoldsIsNotDiagnosed(string assemblyName, string declarations, string members, string body) + { + GeneratorRunResult result = Run(Shader(declarations, members, body), assemblyName); + + Assert.IsTrue(result.Diagnostics.IsEmpty, string.Join(", ", result.Diagnostics.Select(static diagnostic => diagnostic.Id))); + Assert.AreNotEqual(0, GeneratorHelper.GetGeneratedSource(result.GeneratedSources, "Shaders.Shader").Length); + } + + private static string Shader(string declarations, string members, string body) + { + return $$""" + using ComputeWeave; + + namespace Shaders; + + {{declarations}} + + [ThreadGroupSize(DefaultThreadGroupSizes.X)] + [GeneratedComputeShaderDescriptor] + internal readonly partial struct Shader : IComputeShader + { + {{members}} + + private readonly ReadWriteBuffer buffer; + + public void Execute() + { + {{body}} + } + } + """; + } + + private static void AssertReportsOnly(string source, string assemblyName, string expectedId) + { + ImmutableArray diagnostics = Run(source, assemblyName).Diagnostics; + + // Not made distinct, so that one cause reported twice fails rather than reading as one report + string[] actualIds = [.. diagnostics.Select(static diagnostic => diagnostic.Id).Order()]; + + Assert.IsTrue(actualIds.SequenceEqual([expectedId]), $"{expectedId} is not the only report: {string.Join(", ", actualIds)}"); + } + + private static GeneratorRunResult Run(string source, string assemblyName) + { + CSharpCompilation compilation = CompilationHelper.CreateCompilation( + [source], + assemblyName, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, allowUnsafe: true)); + GeneratorDriver driver = GeneratorHelper.CreateDriver(new ComputeShaderDescriptorGenerator()); + GeneratorRunResult result = driver.RunGenerators(compilation).GetRunResult().Results[0]; + + // A fault discards the output for the whole compilation unit, so it is read ahead of the reports + Assert.IsNull(result.Exception, result.Exception?.ToString()); + + return result; + } +}