From b6bcc1c6ccd28df9cf58fc57dd7187c06a3688ab Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 16 Jun 2024 19:32:08 -0700 Subject: [PATCH 1/2] Make 'AllowsUnsafeBlocks' analyzer target attributes --- ...owUnsafeBlocksCompilationOptionAnalyzer.cs | 2 +- .../Diagnostics/DiagnosticDescriptors.cs | 9 ++--- ...safeBlocksCompilationOptionAnalyzerBase.cs | 38 ++++++++++++++++--- ...owUnsafeBlocksCompilationOptionAnalyzer.cs | 2 +- .../Diagnostics/DiagnosticDescriptors.cs | 9 ++--- 5 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/ComputeSharp.D2D1.SourceGenerators/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzer.cs b/src/ComputeSharp.D2D1.SourceGenerators/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzer.cs index 54cf04c64..82cea957b 100644 --- a/src/ComputeSharp.D2D1.SourceGenerators/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzer.cs +++ b/src/ComputeSharp.D2D1.SourceGenerators/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzer.cs @@ -13,7 +13,7 @@ public sealed class MissingAllowUnsafeBlocksCompilationOptionAnalyzer : MissingA /// Creates a new instance. /// public MissingAllowUnsafeBlocksCompilationOptionAnalyzer() - : base(MissingAllowUnsafeBlocksOption) + : base(MissingAllowUnsafeBlocksOption, "ComputeSharp.D2D1.D2DGeneratedPixelShaderDescriptorAttribute") { } } \ No newline at end of file diff --git a/src/ComputeSharp.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/src/ComputeSharp.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index 23d1d418e..83836dddf 100644 --- a/src/ComputeSharp.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/src/ComputeSharp.D2D1.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -944,19 +944,18 @@ partial class DiagnosticDescriptors /// /// Gets a for when the AllowUnsafeBlocks option is not set. /// - /// Format: "Unsafe blocks must be enabled for the source generators to emit valid code (add <AllowUnsafeBlocks>true</AllowUnsafeBlocks> to your .csproj/.props file)". + /// Format: "Using [D2DGeneratedPixelShaderDescriptor] requires unsafe blocks being enabled, as they needed by the source generators to emit valid code (add <AllowUnsafeBlocks>true</AllowUnsafeBlocks> to your .csproj/.props file)". /// /// public static readonly DiagnosticDescriptor MissingAllowUnsafeBlocksOption = new( id: "CMPSD2D0064", title: "Missing 'AllowUnsafeBlocks' compilation option", - messageFormat: "Unsafe blocks must be enabled for the source generators to emit valid code (add true to your .csproj/.props file)", + messageFormat: "Using [D2DGeneratedPixelShaderDescriptor] requires unsafe blocks being enabled, as they needed by the source generators to emit valid code (add true to your .csproj/.props file)", category: "ComputeSharp.D2D1.Shaders", defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true, - description: "Unsafe blocks must be enabled for the source generators to emit valid code (the true option must be set in the .csproj/.props file).", - helpLinkUri: "https://github.com/Sergio0694/ComputeSharp", - customTags: WellKnownDiagnosticTags.CompilationEnd); + description: "Unsafe blocks must be enabled when using [D2DGeneratedPixelShaderDescriptor] for the source generators to emit valid code (the true option must be set in the .csproj/.props file).", + helpLinkUri: "https://github.com/Sergio0694/ComputeSharp"); /// /// Gets a for when a pixel shader type doesn't have an associated descriptor. diff --git a/src/ComputeSharp.SourceGeneration.Hlsl/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzerBase.cs b/src/ComputeSharp.SourceGeneration.Hlsl/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzerBase.cs index 9fe78f1e0..a683fb271 100644 --- a/src/ComputeSharp.SourceGeneration.Hlsl/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzerBase.cs +++ b/src/ComputeSharp.SourceGeneration.Hlsl/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzerBase.cs @@ -9,7 +9,10 @@ namespace ComputeSharp.SourceGeneration.Diagnostics; /// A diagnostic analyzer that generates an error if the AllowUnsafeBlocks compilation option is not set. /// /// The instance to use. -public abstract class MissingAllowUnsafeBlocksCompilationOptionAnalyzerBase(DiagnosticDescriptor diagnosticDescriptor) : DiagnosticAnalyzer +/// The fully qualified type name of the target attribute. +public abstract class MissingAllowUnsafeBlocksCompilationOptionAnalyzerBase( + DiagnosticDescriptor diagnosticDescriptor, + string generatedShaderDescriptorFullyQualifiedTypeName) : DiagnosticAnalyzer { /// public sealed override ImmutableArray SupportedDiagnostics { get; } = [diagnosticDescriptor]; @@ -20,13 +23,38 @@ public sealed override void Initialize(AnalysisContext context) context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); context.EnableConcurrentExecution(); - context.RegisterCompilationAction(context => + context.RegisterCompilationStartAction(context => { - // Check whether unsafe blocks are available, and emit an error if they are not - if (!context.Compilation.IsAllowUnsafeBlocksEnabled()) + // If unsafe blocks are allowed, we'll never need to emit a diagnostic + if (context.Compilation.IsAllowUnsafeBlocksEnabled()) { - context.ReportDiagnostic(Diagnostic.Create(diagnosticDescriptor, location: null)); + return; } + + // Get the symbol for the target attribute type + if (context.Compilation.GetTypeByMetadataName(generatedShaderDescriptorFullyQualifiedTypeName) is not { } generatedShaderDescriptorAttributeSymbol) + { + return; + } + + // Check if any types in the compilation are using the trigger attribute + context.RegisterSymbolAction(context => + { + // Only struct types are possible targets + if (context.Symbol is not INamedTypeSymbol { TypeKind: TypeKind.Struct } typeSymbol) + { + return; + } + + // If the target type is not using the attribute, there's nothing left to do + if (!typeSymbol.TryGetAttributeWithType(generatedShaderDescriptorAttributeSymbol, out AttributeData? attribute)) + { + return; + } + + // Emit the error on the attribute use + context.ReportDiagnostic(Diagnostic.Create(diagnosticDescriptor, attribute.GetLocation())); + }, SymbolKind.NamedType); }); } } \ No newline at end of file diff --git a/src/ComputeSharp.SourceGenerators/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzer.cs b/src/ComputeSharp.SourceGenerators/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzer.cs index 2473abff3..12cfb544c 100644 --- a/src/ComputeSharp.SourceGenerators/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzer.cs +++ b/src/ComputeSharp.SourceGenerators/Diagnostics/Analyzers/MissingAllowUnsafeBlocksCompilationOptionAnalyzer.cs @@ -13,7 +13,7 @@ public sealed class MissingAllowUnsafeBlocksCompilationOptionAnalyzer : MissingA /// Creates a new instance. /// public MissingAllowUnsafeBlocksCompilationOptionAnalyzer() - : base(MissingAllowUnsafeBlocksOption) + : base(MissingAllowUnsafeBlocksOption, "ComputeSharp.GeneratedComputeShaderDescriptorAttribute") { } } \ No newline at end of file diff --git a/src/ComputeSharp.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/src/ComputeSharp.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index 077458cb4..e55d2a4d9 100644 --- a/src/ComputeSharp.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/src/ComputeSharp.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -720,19 +720,18 @@ partial class DiagnosticDescriptors /// /// Gets a for when the AllowUnsafeBlocks option is not set. /// - /// Format: "Unsafe blocks must be enabled for the source generators to emit valid code (add <AllowUnsafeBlocks>true</AllowUnsafeBlocks> to your .csproj/.props file)". + /// Format: "Using [GeneratedComputeShaderDescriptor] requires unsafe blocks being enabled, as they needed by the source generators to emit valid code (add <AllowUnsafeBlocks>true</AllowUnsafeBlocks> to your .csproj/.props file)". /// /// public static readonly DiagnosticDescriptor MissingAllowUnsafeBlocksOption = new( id: "CMPS0052", title: "Missing 'AllowUnsafeBlocks' compilation option", - messageFormat: "Unsafe blocks must be enabled for the source generators to emit valid code (add true to your .csproj/.props file)", + messageFormat: "Using [GeneratedComputeShaderDescriptor] requires unsafe blocks being enabled, as they needed by the source generators to emit valid code (add true to your .csproj/.props file)", category: "ComputeSharp.Shaders", defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true, - description: "Unsafe blocks must be enabled for the source generators to emit valid code (the true option must be set in the .csproj/.props file).", - helpLinkUri: "https://github.com/Sergio0694/ComputeSharp", - customTags: WellKnownDiagnosticTags.CompilationEnd); + description: "Unsafe blocks must be enabled when using [GeneratedComputeShaderDescriptor] for the source generators to emit valid code (the true option must be set in the .csproj/.props file).", + helpLinkUri: "https://github.com/Sergio0694/ComputeSharp"); /// /// Gets a for when a compute shader type doesn't have an associated descriptor. From 2a9f6229bc899290adcd4b15b24676273389755a Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 16 Jun 2024 19:32:26 -0700 Subject: [PATCH 2/2] Add tests for 'AllowsUnsafeBlocks' analyzer --- ...ixelShaderDescriptorGenerator_Analyzers.cs | 24 +++++++++++++++++-- ...lyzerWithLanguageVersionTest{TAnalyzer}.cs | 23 +++++++++++++++--- ...puteShaderDescriptorGenerator_Analyzers.cs | 21 ++++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/tests/ComputeSharp.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Analyzers.cs b/tests/ComputeSharp.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Analyzers.cs index 12cf8f56e..d715ee7e9 100644 --- a/tests/ComputeSharp.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Analyzers.cs +++ b/tests/ComputeSharp.D2D1.Tests.SourceGenerators/Test_D2DPixelShaderDescriptorGenerator_Analyzers.cs @@ -9,7 +9,27 @@ namespace ComputeSharp.D2D1.Tests.SourceGenerators; public class Test_D2DPixelShaderDescriptorGenerator_Analyzers { [TestMethod] - public async Task MissingD2DPixelShaderDescriptor_ComputeShader() + public async Task AllowsUnsafeBlocksNotEnabled_Warns() + { + const string source = """ + using ComputeSharp; + using ComputeSharp.D2D1; + + [{|CMPSD2D0064:D2DGeneratedPixelShaderDescriptor|}] + internal partial struct MyShader : ID2D1PixelShader + { + public Float4 Execute() + { + return 0; + } + } + """; + + await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source, allowUnsafeBlocks: false); + } + + [TestMethod] + public async Task MissingD2DPixelShaderDescriptor_Warns() { const string source = """ using ComputeSharp; @@ -28,7 +48,7 @@ public Float4 Execute() } [TestMethod] - public async Task MissingComputeShaderDescriptor_ManuallyImplemented_DoesNotWarn() + public async Task MissingD2DPixelShaderDescriptor_ManuallyImplemented_DoesNotWarn() { const string source = """ using System; diff --git a/tests/ComputeSharp.Tests.SourceGenerators/Helpers/CSharpAnalyzerWithLanguageVersionTest{TAnalyzer}.cs b/tests/ComputeSharp.Tests.SourceGenerators/Helpers/CSharpAnalyzerWithLanguageVersionTest{TAnalyzer}.cs index 58304f3b9..82e35d1df 100644 --- a/tests/ComputeSharp.Tests.SourceGenerators/Helpers/CSharpAnalyzerWithLanguageVersionTest{TAnalyzer}.cs +++ b/tests/ComputeSharp.Tests.SourceGenerators/Helpers/CSharpAnalyzerWithLanguageVersionTest{TAnalyzer}.cs @@ -23,6 +23,11 @@ namespace ComputeSharp.Tests.SourceGenerators.Helpers; internal sealed class CSharpAnalyzerWithLanguageVersionTest : CSharpAnalyzerTest where TAnalyzer : DiagnosticAnalyzer, new() { + /// + /// Whether to enable unsafe blocks. + /// + private readonly bool allowUnsafeBlocks; + /// /// The C# language version to use to parse code. /// @@ -31,12 +36,20 @@ internal sealed class CSharpAnalyzerWithLanguageVersionTest : CSharpA /// /// Creates a new instance with the specified paramaters. /// + /// Whether to enable unsafe blocks. /// The C# language version to use to parse code. - private CSharpAnalyzerWithLanguageVersionTest(LanguageVersion languageVersion) + private CSharpAnalyzerWithLanguageVersionTest(bool allowUnsafeBlocks, LanguageVersion languageVersion) { + this.allowUnsafeBlocks = allowUnsafeBlocks; this.languageVersion = languageVersion; } + /// + protected override CompilationOptions CreateCompilationOptions() + { + return new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, allowUnsafe: this.allowUnsafeBlocks); + } + /// protected override ParseOptions CreateParseOptions() { @@ -45,10 +58,14 @@ protected override ParseOptions CreateParseOptions() /// /// The source code to analyze. + /// Whether to enable unsafe blocks. /// The language version to use to run the test. - public static Task VerifyAnalyzerAsync(string source, LanguageVersion languageVersion = LanguageVersion.CSharp12) + public static Task VerifyAnalyzerAsync( + string source, + bool allowUnsafeBlocks = true, + LanguageVersion languageVersion = LanguageVersion.CSharp12) { - CSharpAnalyzerWithLanguageVersionTest test = new(languageVersion) { TestCode = source }; + CSharpAnalyzerWithLanguageVersionTest test = new(allowUnsafeBlocks, languageVersion) { TestCode = source }; test.TestState.ReferenceAssemblies = ReferenceAssemblies.Net.Net80; test.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Core::ComputeSharp.Hlsl).Assembly.Location)); diff --git a/tests/ComputeSharp.Tests.SourceGenerators/Test_ComputeShaderDescriptorGenerator_Analyzers.cs b/tests/ComputeSharp.Tests.SourceGenerators/Test_ComputeShaderDescriptorGenerator_Analyzers.cs index 4ead59b9d..67f65a418 100644 --- a/tests/ComputeSharp.Tests.SourceGenerators/Test_ComputeShaderDescriptorGenerator_Analyzers.cs +++ b/tests/ComputeSharp.Tests.SourceGenerators/Test_ComputeShaderDescriptorGenerator_Analyzers.cs @@ -8,6 +8,27 @@ namespace ComputeSharp.Tests.SourceGenerators; [TestClass] public class Test_ComputeShaderDescriptorGenerator_Analyzers { + [TestMethod] + public async Task AllowsUnsafeBlocksNotEnabled_Warns() + { + const string source = """ + using ComputeSharp; + + [ThreadGroupSize(DefaultThreadGroupSizes.X)] + [{|CMPS0052:GeneratedComputeShaderDescriptor|}] + internal readonly partial struct MyShader : IComputeShader + { + private readonly ReadWriteBuffer buffer; + + public void Execute() + { + } + } + """; + + await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source, allowUnsafeBlocks: false); + } + [TestMethod] public async Task MissingComputeShaderDescriptor_ComputeShader() {