Skip to content

Commit

Permalink
Merge pull request #816 from Sergio0694/dev/improved-unsafe-blocks-an…
Browse files Browse the repository at this point in the history
…alyzer

Make 'AllowsUnsafeBlocks' analyzer only trigger on attribute uses
  • Loading branch information
Sergio0694 authored Jun 17, 2024
2 parents ebee50d + 2a9f622 commit 98961f8
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public sealed class MissingAllowUnsafeBlocksCompilationOptionAnalyzer : MissingA
/// Creates a new <see cref="MissingAllowUnsafeBlocksCompilationOptionAnalyzer"/> instance.
/// </summary>
public MissingAllowUnsafeBlocksCompilationOptionAnalyzer()
: base(MissingAllowUnsafeBlocksOption)
: base(MissingAllowUnsafeBlocksOption, "ComputeSharp.D2D1.D2DGeneratedPixelShaderDescriptorAttribute")
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -944,19 +944,18 @@ partial class DiagnosticDescriptors
/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> for when the <c>AllowUnsafeBlocks</c> option is not set.
/// <para>
/// Format: <c>"Unsafe blocks must be enabled for the source generators to emit valid code (add &lt;AllowUnsafeBlocks&gt;true&lt;/AllowUnsafeBlocks&gt; to your .csproj/.props file)"</c>.
/// Format: <c>"Using [D2DGeneratedPixelShaderDescriptor] requires unsafe blocks being enabled, as they needed by the source generators to emit valid code (add &lt;AllowUnsafeBlocks&gt;true&lt;/AllowUnsafeBlocks&gt; to your .csproj/.props file)"</c>.
/// </para>
/// </summary>
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 <AllowUnsafeBlocks>true</AllowUnsafeBlocks> 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 <AllowUnsafeBlocks>true</AllowUnsafeBlocks> 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 <AllowUnsafeBlocks>true</AllowUnsafeBlocks> 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 <AllowUnsafeBlocks>true</AllowUnsafeBlocks> option must be set in the .csproj/.props file).",
helpLinkUri: "https://github.com/Sergio0694/ComputeSharp");

/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> for when a pixel shader type doesn't have an associated descriptor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ namespace ComputeSharp.SourceGeneration.Diagnostics;
/// A diagnostic analyzer that generates an error if the <c>AllowUnsafeBlocks</c> compilation option is not set.
/// </summary>
/// <param name="diagnosticDescriptor">The <see cref="DiagnosticDescriptor"/> instance to use.</param>
public abstract class MissingAllowUnsafeBlocksCompilationOptionAnalyzerBase(DiagnosticDescriptor diagnosticDescriptor) : DiagnosticAnalyzer
/// <param name="generatedShaderDescriptorFullyQualifiedTypeName">The fully qualified type name of the target attribute.</param>
public abstract class MissingAllowUnsafeBlocksCompilationOptionAnalyzerBase(
DiagnosticDescriptor diagnosticDescriptor,
string generatedShaderDescriptorFullyQualifiedTypeName) : DiagnosticAnalyzer
{
/// <inheritdoc/>
public sealed override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = [diagnosticDescriptor];
Expand All @@ -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);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public sealed class MissingAllowUnsafeBlocksCompilationOptionAnalyzer : MissingA
/// Creates a new <see cref="MissingAllowUnsafeBlocksCompilationOptionAnalyzer"/> instance.
/// </summary>
public MissingAllowUnsafeBlocksCompilationOptionAnalyzer()
: base(MissingAllowUnsafeBlocksOption)
: base(MissingAllowUnsafeBlocksOption, "ComputeSharp.GeneratedComputeShaderDescriptorAttribute")
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -720,19 +720,18 @@ partial class DiagnosticDescriptors
/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> for when the <c>AllowUnsafeBlocks</c> option is not set.
/// <para>
/// Format: <c>"Unsafe blocks must be enabled for the source generators to emit valid code (add &lt;AllowUnsafeBlocks&gt;true&lt;/AllowUnsafeBlocks&gt; to your .csproj/.props file)"</c>.
/// Format: <c>"Using [GeneratedComputeShaderDescriptor] requires unsafe blocks being enabled, as they needed by the source generators to emit valid code (add &lt;AllowUnsafeBlocks&gt;true&lt;/AllowUnsafeBlocks&gt; to your .csproj/.props file)"</c>.
/// </para>
/// </summary>
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 <AllowUnsafeBlocks>true</AllowUnsafeBlocks> 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 <AllowUnsafeBlocks>true</AllowUnsafeBlocks> 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 <AllowUnsafeBlocks>true</AllowUnsafeBlocks> 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 <AllowUnsafeBlocks>true</AllowUnsafeBlocks> option must be set in the .csproj/.props file).",
helpLinkUri: "https://github.com/Sergio0694/ComputeSharp");

/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> for when a compute shader type doesn't have an associated descriptor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<MissingAllowUnsafeBlocksCompilationOptionAnalyzer>.VerifyAnalyzerAsync(source, allowUnsafeBlocks: false);
}

[TestMethod]
public async Task MissingD2DPixelShaderDescriptor_Warns()
{
const string source = """
using ComputeSharp;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ namespace ComputeSharp.Tests.SourceGenerators.Helpers;
internal sealed class CSharpAnalyzerWithLanguageVersionTest<TAnalyzer> : CSharpAnalyzerTest<TAnalyzer, MSTestVerifier>
where TAnalyzer : DiagnosticAnalyzer, new()
{
/// <summary>
/// Whether to enable unsafe blocks.
/// </summary>
private readonly bool allowUnsafeBlocks;

/// <summary>
/// The C# language version to use to parse code.
/// </summary>
Expand All @@ -31,12 +36,20 @@ internal sealed class CSharpAnalyzerWithLanguageVersionTest<TAnalyzer> : CSharpA
/// <summary>
/// Creates a new <see cref="CSharpAnalyzerWithLanguageVersionTest{TAnalyzer}"/> instance with the specified paramaters.
/// </summary>
/// <param name="allowUnsafeBlocks">Whether to enable unsafe blocks.</param>
/// <param name="languageVersion">The C# language version to use to parse code.</param>
private CSharpAnalyzerWithLanguageVersionTest(LanguageVersion languageVersion)
private CSharpAnalyzerWithLanguageVersionTest(bool allowUnsafeBlocks, LanguageVersion languageVersion)
{
this.allowUnsafeBlocks = allowUnsafeBlocks;
this.languageVersion = languageVersion;
}

/// <inheritdoc/>
protected override CompilationOptions CreateCompilationOptions()
{
return new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, allowUnsafe: this.allowUnsafeBlocks);
}

/// <inheritdoc/>
protected override ParseOptions CreateParseOptions()
{
Expand All @@ -45,10 +58,14 @@ protected override ParseOptions CreateParseOptions()

/// <inheritdoc cref="AnalyzerVerifier{TAnalyzer, TTest, TVerifier}.VerifyAnalyzerAsync"/>
/// <param name="source">The source code to analyze.</param>
/// <param name="allowUnsafeBlocks">Whether to enable unsafe blocks.</param>
/// <param name="languageVersion">The language version to use to run the test.</param>
public static Task VerifyAnalyzerAsync(string source, LanguageVersion languageVersion = LanguageVersion.CSharp12)
public static Task VerifyAnalyzerAsync(
string source,
bool allowUnsafeBlocks = true,
LanguageVersion languageVersion = LanguageVersion.CSharp12)
{
CSharpAnalyzerWithLanguageVersionTest<TAnalyzer> test = new(languageVersion) { TestCode = source };
CSharpAnalyzerWithLanguageVersionTest<TAnalyzer> test = new(allowUnsafeBlocks, languageVersion) { TestCode = source };

test.TestState.ReferenceAssemblies = ReferenceAssemblies.Net.Net80;
test.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Core::ComputeSharp.Hlsl).Assembly.Location));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<float> buffer;

public void Execute()
{
}
}
""";

await CSharpAnalyzerWithLanguageVersionTest<MissingAllowUnsafeBlocksCompilationOptionAnalyzer>.VerifyAnalyzerAsync(source, allowUnsafeBlocks: false);
}

[TestMethod]
public async Task MissingComputeShaderDescriptor_ComputeShader()
{
Expand Down

0 comments on commit 98961f8

Please sign in to comment.