Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using System.Numerics.Hashing;
using Microsoft.CodeAnalysis;

namespace SourceGenerators;

/// <summary>
/// Descriptor for diagnostic instances using structural equality comparison.
/// Provides a work-around for https://github.com/dotnet/roslyn/issues/68291.
/// </summary>
internal readonly struct DiagnosticInfo : IEquatable<DiagnosticInfo>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, I believe the regex generator defines such a struct as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance this seems like a type that would be defined as a record if arrays were naturally equitable. Why wasn't this just defined as a record with ImmutableEquatableArray<object?> wrapping MessageArgs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would force an intermediate allocation from object[] to ImmutableEquatableArray then back to an object[] when the Diagnostic gets materialized. I think it's small enough to justify a bespoke type. Longer term though we should be looking at addressing dotnet/roslyn#68291 and use Diagnostic in the equatable models directly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model. Location has a reference to the SyntaxTree that it was created from, and that will change every time the file changes. This wrapper has similar issues: even if message args compared correctly, it would still be broken for equality.

@layomialayomiaSep 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model.

Per #89587 (comment) this bit is mitigated for this PR, follow up in #92509.

{
public DiagnosticDescriptor Descriptor { get; private init; }
public object?[] MessageArgs { get; private init; }
public Location? Location { get; private init; }

public static DiagnosticInfo Create(DiagnosticDescriptor descriptor, Location? location, object?[]? messageArgs)
{
Location? trimmedLocation = location is null ? null : GetTrimmedLocation(location);

return new DiagnosticInfo
{
Descriptor = descriptor,
Location = trimmedLocation,
MessageArgs = messageArgs ?? Array.Empty<object?>()
};

// Creates a copy of the Location instance that does not capture a reference to Compilation.
static Location GetTrimmedLocation(Location location)
=> Location.Create(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span);
}

public Diagnostic CreateDiagnostic()
=> Diagnostic.Create(Descriptor, Location, MessageArgs);

public override readonly bool Equals(object? obj) => obj is DiagnosticInfo info && Equals(info);

public readonly bool Equals(DiagnosticInfo other)
{
return Descriptor.Equals(other.Descriptor) &&
MessageArgs.SequenceEqual(other.MessageArgs) &&
Location == other.Location;
}

public override readonly int GetHashCode()
{
int hashCode = Descriptor.GetHashCode();
foreach (object? messageArg in MessageArgs)
{
hashCode = HashHelpers.Combine(hashCode, messageArg?.GetHashCode() ?? 0);
}

hashCode = HashHelpers.Combine(hashCode, Location?.GetHashCode() ?? 0);
return hashCode;
}
}
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics.Hashing;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// Provides an immutable list implementation which implements sequence equality.
Expand DownExpand Up@@ -72,15 +73,9 @@ public bool MoveNext()
}
}

public static class ImmutableEquatableArray
internal static class ImmutableEquatableArray
Comment thread
layomia marked this conversation as resolved.
Outdated
{
public static ImmutableEquatableArray<T> Empty<T>() where T : IEquatable<T>
=> ImmutableEquatableArray<T>.Empty;

public static ImmutableEquatableArray<T> ToImmutableEquatableArray<T>(this IEnumerable<T> values) where T : IEquatable<T>
=> new(values);

public static ImmutableEquatableArray<T> Create<T>(params T[] values) where T : IEquatable<T>
=> values is { Length: > 0 } ? new(values) : ImmutableEquatableArray<T>.Empty;
}
}
4 changes: 4 additions & 0 deletions src/libraries/Common/src/SourceGenerators/TypeModelHelper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,8 @@

using Microsoft.CodeAnalysis;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace SourceGenerators
{
Expand DownExpand Up@@ -32,5 +34,7 @@ void TraverseContainingTypes(INamedTypeSymbol current)
}
}
}

public static string GetFullyQualifiedName(this ITypeSymbol type) => type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
}
}
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using Microsoft.CodeAnalysis;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// An equatable value representing type identity.
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Xunit;

namespace SourceGenerators.Tests
{
internal static class GeneratorTestHelpers
{
/// <summary>
/// Asserts for structural equality, returning a path to the mismatching data when not equal.
/// </summary>
public static void AssertStructurallyEqual<T>(T expected, T actual)
{
CheckAreEqualCore(expected, actual, new());
static void CheckAreEqualCore(object expected, object actual, Stack<string> path)
{
if (expected is null || actual is null)
{
if (expected is not null || actual is not null)
{
FailNotEqual();
}

return;
}

Type type = expected.GetType();
if (type != actual.GetType())
{
FailNotEqual();
return;
}

if (expected is IEnumerable leftCollection)
{
if (actual is not IEnumerable rightCollection)
{
FailNotEqual();
return;
}

object?[] expectedValues = leftCollection.Cast<object?>().ToArray();
object?[] actualValues = rightCollection.Cast<object?>().ToArray();

for (int i = 0; i < Math.Max(expectedValues.Length, actualValues.Length); i++)
{
object? expectedElement = i < expectedValues.Length ? expectedValues[i] : "<end of collection>";
object? actualElement = i < actualValues.Length ? actualValues[i] : "<end of collection>";

path.Push($"[{i}]");
CheckAreEqualCore(expectedElement, actualElement, path);
path.Pop();
}
}

if (type.GetProperty("EqualityContract", BindingFlags.Instance | BindingFlags.NonPublic, null, returnType: typeof(Type), types: Array.Empty<Type>(), null) != null)
{
// Type is a C# record, run pointwise equality comparison.
foreach (PropertyInfo property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
path.Push("." + property.Name);
CheckAreEqualCore(property.GetValue(expected), property.GetValue(actual), path);
path.Pop();
}

return;
}

if (!expected.Equals(actual))
{
FailNotEqual();
}

void FailNotEqual() => Assert.Fail($"Value not equal in ${string.Join("", path.Reverse())}: expected {expected}, but was {actual}.");
}
}
}
}
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using SourceGenerators;

Expand All@@ -11,19 +10,22 @@ public sealed partial class ConfigurationBindingGenerator : IIncrementalGenerato
{
private sealed partial class Emitter
{
private readonly SourceProductionContext _context;
private readonly SourceGenerationSpec _sourceGenSpec;
private readonly InterceptorInfo _interceptorInfo;
private readonly BindingHelperInfo _bindingHelperInfo;
private readonly TypeIndex _typeIndex;

private readonly SourceWriter _writer = new();

public Emitter(SourceProductionContext context, SourceGenerationSpec sourceGenSpec)
public Emitter(SourceGenerationSpec sourceGenSpec)
{
_context = context;
_sourceGenSpec = sourceGenSpec;
_interceptorInfo = sourceGenSpec.InterceptorInfo;
_bindingHelperInfo = sourceGenSpec.BindingHelperInfo;
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
}

public void Emit()
public void Emit(SourceProductionContext context)
{
if (!ShouldEmitBindingExtensions())
if (!ShouldEmitMethods(MethodsToGen.Any))
{
return;
}
Expand DownExpand Up@@ -52,7 +54,7 @@ file static class {{Identifier.BindingExtensions}}

EmitEndBlock(); // Binding namespace.

_context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
}

private void EmitInterceptsLocationAttrDecl()
Expand All@@ -79,7 +81,7 @@ public InterceptsLocationAttribute(string filePath, int line, int column)

private void EmitUsingStatements()
{
foreach (string @namespace in _sourceGenSpec.Namespaces.ToImmutableSortedSet())
foreach (string @namespace in _bindingHelperInfo.Namespaces)
{
_writer.WriteLine($"using {@namespace};");
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Make config binding gen incremental by layomia · Pull Request #89587 · dotnet/runtime · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using System.Numerics.Hashing;
using Microsoft.CodeAnalysis;

namespace SourceGenerators;

/// <summary>
/// Descriptor for diagnostic instances using structural equality comparison.
/// Provides a work-around for https://github.com/dotnet/roslyn/issues/68291.
/// </summary>
internal readonly struct DiagnosticInfo : IEquatable<DiagnosticInfo>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, I believe the regex generator defines such a struct as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance this seems like a type that would be defined as a record if arrays were naturally equitable. Why wasn't this just defined as a record with ImmutableEquatableArray<object?> wrapping MessageArgs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would force an intermediate allocation from object[] to ImmutableEquatableArray then back to an object[] when the Diagnostic gets materialized. I think it's small enough to justify a bespoke type. Longer term though we should be looking at addressing dotnet/roslyn#68291 and use Diagnostic in the equatable models directly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model. Location has a reference to the SyntaxTree that it was created from, and that will change every time the file changes. This wrapper has similar issues: even if message args compared correctly, it would still be broken for equality.

@layomialayomiaSep 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model.

Per #89587 (comment) this bit is mitigated for this PR, follow up in #92509.

{
public DiagnosticDescriptor Descriptor { get; private init; }
public object?[] MessageArgs { get; private init; }
public Location? Location { get; private init; }

public static DiagnosticInfo Create(DiagnosticDescriptor descriptor, Location? location, object?[]? messageArgs)
{
Location? trimmedLocation = location is null ? null : GetTrimmedLocation(location);

return new DiagnosticInfo
{
Descriptor = descriptor,
Location = trimmedLocation,
MessageArgs = messageArgs ?? Array.Empty<object?>()
};

// Creates a copy of the Location instance that does not capture a reference to Compilation.
static Location GetTrimmedLocation(Location location)
=> Location.Create(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span);
}

public Diagnostic CreateDiagnostic()
=> Diagnostic.Create(Descriptor, Location, MessageArgs);

public override readonly bool Equals(object? obj) => obj is DiagnosticInfo info && Equals(info);

public readonly bool Equals(DiagnosticInfo other)
{
return Descriptor.Equals(other.Descriptor) &&
MessageArgs.SequenceEqual(other.MessageArgs) &&
Location == other.Location;
}

public override readonly int GetHashCode()
{
int hashCode = Descriptor.GetHashCode();
foreach (object? messageArg in MessageArgs)
{
hashCode = HashHelpers.Combine(hashCode, messageArg?.GetHashCode() ?? 0);
}

hashCode = HashHelpers.Combine(hashCode, Location?.GetHashCode() ?? 0);
return hashCode;
}
}
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics.Hashing;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// Provides an immutable list implementation which implements sequence equality.
Expand DownExpand Up@@ -72,15 +73,9 @@ public bool MoveNext()
}
}

public static class ImmutableEquatableArray
internal static class ImmutableEquatableArray
Comment thread
layomia marked this conversation as resolved.
Outdated
{
public static ImmutableEquatableArray<T> Empty<T>() where T : IEquatable<T>
=> ImmutableEquatableArray<T>.Empty;

public static ImmutableEquatableArray<T> ToImmutableEquatableArray<T>(this IEnumerable<T> values) where T : IEquatable<T>
=> new(values);

public static ImmutableEquatableArray<T> Create<T>(params T[] values) where T : IEquatable<T>
=> values is { Length: > 0 } ? new(values) : ImmutableEquatableArray<T>.Empty;
}
}
4 changes: 4 additions & 0 deletions src/libraries/Common/src/SourceGenerators/TypeModelHelper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,8 @@

using Microsoft.CodeAnalysis;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace SourceGenerators
{
Expand DownExpand Up@@ -32,5 +34,7 @@ void TraverseContainingTypes(INamedTypeSymbol current)
}
}
}

public static string GetFullyQualifiedName(this ITypeSymbol type) => type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
}
}
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using Microsoft.CodeAnalysis;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// An equatable value representing type identity.
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Xunit;

namespace SourceGenerators.Tests
{
internal static class GeneratorTestHelpers
{
/// <summary>
/// Asserts for structural equality, returning a path to the mismatching data when not equal.
/// </summary>
public static void AssertStructurallyEqual<T>(T expected, T actual)
{
CheckAreEqualCore(expected, actual, new());
static void CheckAreEqualCore(object expected, object actual, Stack<string> path)
{
if (expected is null || actual is null)
{
if (expected is not null || actual is not null)
{
FailNotEqual();
}

return;
}

Type type = expected.GetType();
if (type != actual.GetType())
{
FailNotEqual();
return;
}

if (expected is IEnumerable leftCollection)
{
if (actual is not IEnumerable rightCollection)
{
FailNotEqual();
return;
}

object?[] expectedValues = leftCollection.Cast<object?>().ToArray();
object?[] actualValues = rightCollection.Cast<object?>().ToArray();

for (int i = 0; i < Math.Max(expectedValues.Length, actualValues.Length); i++)
{
object? expectedElement = i < expectedValues.Length ? expectedValues[i] : "<end of collection>";
object? actualElement = i < actualValues.Length ? actualValues[i] : "<end of collection>";

path.Push($"[{i}]");
CheckAreEqualCore(expectedElement, actualElement, path);
path.Pop();
}
}

if (type.GetProperty("EqualityContract", BindingFlags.Instance | BindingFlags.NonPublic, null, returnType: typeof(Type), types: Array.Empty<Type>(), null) != null)
{
// Type is a C# record, run pointwise equality comparison.
foreach (PropertyInfo property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
path.Push("." + property.Name);
CheckAreEqualCore(property.GetValue(expected), property.GetValue(actual), path);
path.Pop();
}

return;
}

if (!expected.Equals(actual))
{
FailNotEqual();
}

void FailNotEqual() => Assert.Fail($"Value not equal in ${string.Join("", path.Reverse())}: expected {expected}, but was {actual}.");
}
}
}
}
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using SourceGenerators;

Expand All@@ -11,19 +10,22 @@ public sealed partial class ConfigurationBindingGenerator : IIncrementalGenerato
{
private sealed partial class Emitter
{
private readonly SourceProductionContext _context;
private readonly SourceGenerationSpec _sourceGenSpec;
private readonly InterceptorInfo _interceptorInfo;
private readonly BindingHelperInfo _bindingHelperInfo;
private readonly TypeIndex _typeIndex;

private readonly SourceWriter _writer = new();

public Emitter(SourceProductionContext context, SourceGenerationSpec sourceGenSpec)
public Emitter(SourceGenerationSpec sourceGenSpec)
{
_context = context;
_sourceGenSpec = sourceGenSpec;
_interceptorInfo = sourceGenSpec.InterceptorInfo;
_bindingHelperInfo = sourceGenSpec.BindingHelperInfo;
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
}

public void Emit()
public void Emit(SourceProductionContext context)
{
if (!ShouldEmitBindingExtensions())
if (!ShouldEmitMethods(MethodsToGen.Any))
{
return;
}
Expand DownExpand Up@@ -52,7 +54,7 @@ file static class {{Identifier.BindingExtensions}}

EmitEndBlock(); // Binding namespace.

_context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
}

private void EmitInterceptsLocationAttrDecl()
Expand All@@ -79,7 +81,7 @@ public InterceptsLocationAttribute(string filePath, int line, int column)

private void EmitUsingStatements()
{
foreach (string @namespace in _sourceGenSpec.Namespaces.ToImmutableSortedSet())
foreach (string @namespace in _bindingHelperInfo.Namespaces)
{
_writer.WriteLine($"using {@namespace};");
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Make config binding gen incremental by layomia · Pull Request #89587 · dotnet/runtime · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using System.Numerics.Hashing;
using Microsoft.CodeAnalysis;

namespace SourceGenerators;

/// <summary>
/// Descriptor for diagnostic instances using structural equality comparison.
/// Provides a work-around for https://github.com/dotnet/roslyn/issues/68291.
/// </summary>
internal readonly struct DiagnosticInfo : IEquatable<DiagnosticInfo>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, I believe the regex generator defines such a struct as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance this seems like a type that would be defined as a record if arrays were naturally equitable. Why wasn't this just defined as a record with ImmutableEquatableArray<object?> wrapping MessageArgs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would force an intermediate allocation from object[] to ImmutableEquatableArray then back to an object[] when the Diagnostic gets materialized. I think it's small enough to justify a bespoke type. Longer term though we should be looking at addressing dotnet/roslyn#68291 and use Diagnostic in the equatable models directly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model. Location has a reference to the SyntaxTree that it was created from, and that will change every time the file changes. This wrapper has similar issues: even if message args compared correctly, it would still be broken for equality.

@layomialayomiaSep 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model.

Per #89587 (comment) this bit is mitigated for this PR, follow up in #92509.

{
public DiagnosticDescriptor Descriptor { get; private init; }
public object?[] MessageArgs { get; private init; }
public Location? Location { get; private init; }

public static DiagnosticInfo Create(DiagnosticDescriptor descriptor, Location? location, object?[]? messageArgs)
{
Location? trimmedLocation = location is null ? null : GetTrimmedLocation(location);

return new DiagnosticInfo
{
Descriptor = descriptor,
Location = trimmedLocation,
MessageArgs = messageArgs ?? Array.Empty<object?>()
};

// Creates a copy of the Location instance that does not capture a reference to Compilation.
static Location GetTrimmedLocation(Location location)
=> Location.Create(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span);
}

public Diagnostic CreateDiagnostic()
=> Diagnostic.Create(Descriptor, Location, MessageArgs);

public override readonly bool Equals(object? obj) => obj is DiagnosticInfo info && Equals(info);

public readonly bool Equals(DiagnosticInfo other)
{
return Descriptor.Equals(other.Descriptor) &&
MessageArgs.SequenceEqual(other.MessageArgs) &&
Location == other.Location;
}

public override readonly int GetHashCode()
{
int hashCode = Descriptor.GetHashCode();
foreach (object? messageArg in MessageArgs)
{
hashCode = HashHelpers.Combine(hashCode, messageArg?.GetHashCode() ?? 0);
}

hashCode = HashHelpers.Combine(hashCode, Location?.GetHashCode() ?? 0);
return hashCode;
}
}
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics.Hashing;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// Provides an immutable list implementation which implements sequence equality.
Expand DownExpand Up@@ -72,15 +73,9 @@ public bool MoveNext()
}
}

public static class ImmutableEquatableArray
internal static class ImmutableEquatableArray
Comment thread
layomia marked this conversation as resolved.
Outdated
{
public static ImmutableEquatableArray<T> Empty<T>() where T : IEquatable<T>
=> ImmutableEquatableArray<T>.Empty;

public static ImmutableEquatableArray<T> ToImmutableEquatableArray<T>(this IEnumerable<T> values) where T : IEquatable<T>
=> new(values);

public static ImmutableEquatableArray<T> Create<T>(params T[] values) where T : IEquatable<T>
=> values is { Length: > 0 } ? new(values) : ImmutableEquatableArray<T>.Empty;
}
}
4 changes: 4 additions & 0 deletions src/libraries/Common/src/SourceGenerators/TypeModelHelper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,8 @@

using Microsoft.CodeAnalysis;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace SourceGenerators
{
Expand DownExpand Up@@ -32,5 +34,7 @@ void TraverseContainingTypes(INamedTypeSymbol current)
}
}
}

public static string GetFullyQualifiedName(this ITypeSymbol type) => type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
}
}
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using Microsoft.CodeAnalysis;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// An equatable value representing type identity.
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Xunit;

namespace SourceGenerators.Tests
{
internal static class GeneratorTestHelpers
{
/// <summary>
/// Asserts for structural equality, returning a path to the mismatching data when not equal.
/// </summary>
public static void AssertStructurallyEqual<T>(T expected, T actual)
{
CheckAreEqualCore(expected, actual, new());
static void CheckAreEqualCore(object expected, object actual, Stack<string> path)
{
if (expected is null || actual is null)
{
if (expected is not null || actual is not null)
{
FailNotEqual();
}

return;
}

Type type = expected.GetType();
if (type != actual.GetType())
{
FailNotEqual();
return;
}

if (expected is IEnumerable leftCollection)
{
if (actual is not IEnumerable rightCollection)
{
FailNotEqual();
return;
}

object?[] expectedValues = leftCollection.Cast<object?>().ToArray();
object?[] actualValues = rightCollection.Cast<object?>().ToArray();

for (int i = 0; i < Math.Max(expectedValues.Length, actualValues.Length); i++)
{
object? expectedElement = i < expectedValues.Length ? expectedValues[i] : "<end of collection>";
object? actualElement = i < actualValues.Length ? actualValues[i] : "<end of collection>";

path.Push($"[{i}]");
CheckAreEqualCore(expectedElement, actualElement, path);
path.Pop();
}
}

if (type.GetProperty("EqualityContract", BindingFlags.Instance | BindingFlags.NonPublic, null, returnType: typeof(Type), types: Array.Empty<Type>(), null) != null)
{
// Type is a C# record, run pointwise equality comparison.
foreach (PropertyInfo property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
path.Push("." + property.Name);
CheckAreEqualCore(property.GetValue(expected), property.GetValue(actual), path);
path.Pop();
}

return;
}

if (!expected.Equals(actual))
{
FailNotEqual();
}

void FailNotEqual() => Assert.Fail($"Value not equal in ${string.Join("", path.Reverse())}: expected {expected}, but was {actual}.");
}
}
}
}
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using SourceGenerators;

Expand All@@ -11,19 +10,22 @@ public sealed partial class ConfigurationBindingGenerator : IIncrementalGenerato
{
private sealed partial class Emitter
{
private readonly SourceProductionContext _context;
private readonly SourceGenerationSpec _sourceGenSpec;
private readonly InterceptorInfo _interceptorInfo;
private readonly BindingHelperInfo _bindingHelperInfo;
private readonly TypeIndex _typeIndex;

private readonly SourceWriter _writer = new();

public Emitter(SourceProductionContext context, SourceGenerationSpec sourceGenSpec)
public Emitter(SourceGenerationSpec sourceGenSpec)
{
_context = context;
_sourceGenSpec = sourceGenSpec;
_interceptorInfo = sourceGenSpec.InterceptorInfo;
_bindingHelperInfo = sourceGenSpec.BindingHelperInfo;
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
}

public void Emit()
public void Emit(SourceProductionContext context)
{
if (!ShouldEmitBindingExtensions())
if (!ShouldEmitMethods(MethodsToGen.Any))
{
return;
}
Expand DownExpand Up@@ -52,7 +54,7 @@ file static class {{Identifier.BindingExtensions}}

EmitEndBlock(); // Binding namespace.

_context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
}

private void EmitInterceptsLocationAttrDecl()
Expand All@@ -79,7 +81,7 @@ public InterceptsLocationAttribute(string filePath, int line, int column)

private void EmitUsingStatements()
{
foreach (string @namespace in _sourceGenSpec.Namespaces.ToImmutableSortedSet())
foreach (string @namespace in _bindingHelperInfo.Namespaces)
{
_writer.WriteLine($"using {@namespace};");
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Make config binding gen incremental by layomia · Pull Request #89587 · dotnet/runtime · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using System.Numerics.Hashing;
using Microsoft.CodeAnalysis;

namespace SourceGenerators;

/// <summary>
/// Descriptor for diagnostic instances using structural equality comparison.
/// Provides a work-around for https://github.com/dotnet/roslyn/issues/68291.
/// </summary>
internal readonly struct DiagnosticInfo : IEquatable<DiagnosticInfo>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, I believe the regex generator defines such a struct as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance this seems like a type that would be defined as a record if arrays were naturally equitable. Why wasn't this just defined as a record with ImmutableEquatableArray<object?> wrapping MessageArgs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would force an intermediate allocation from object[] to ImmutableEquatableArray then back to an object[] when the Diagnostic gets materialized. I think it's small enough to justify a bespoke type. Longer term though we should be looking at addressing dotnet/roslyn#68291 and use Diagnostic in the equatable models directly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model. Location has a reference to the SyntaxTree that it was created from, and that will change every time the file changes. This wrapper has similar issues: even if message args compared correctly, it would still be broken for equality.

@layomialayomiaSep 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model.

Per #89587 (comment) this bit is mitigated for this PR, follow up in #92509.

{
public DiagnosticDescriptor Descriptor { get; private init; }
public object?[] MessageArgs { get; private init; }
public Location? Location { get; private init; }

public static DiagnosticInfo Create(DiagnosticDescriptor descriptor, Location? location, object?[]? messageArgs)
{
Location? trimmedLocation = location is null ? null : GetTrimmedLocation(location);

return new DiagnosticInfo
{
Descriptor = descriptor,
Location = trimmedLocation,
MessageArgs = messageArgs ?? Array.Empty<object?>()
};

// Creates a copy of the Location instance that does not capture a reference to Compilation.
static Location GetTrimmedLocation(Location location)
=> Location.Create(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span);
}

public Diagnostic CreateDiagnostic()
=> Diagnostic.Create(Descriptor, Location, MessageArgs);

public override readonly bool Equals(object? obj) => obj is DiagnosticInfo info && Equals(info);

public readonly bool Equals(DiagnosticInfo other)
{
return Descriptor.Equals(other.Descriptor) &&
MessageArgs.SequenceEqual(other.MessageArgs) &&
Location == other.Location;
}

public override readonly int GetHashCode()
{
int hashCode = Descriptor.GetHashCode();
foreach (object? messageArg in MessageArgs)
{
hashCode = HashHelpers.Combine(hashCode, messageArg?.GetHashCode() ?? 0);
}

hashCode = HashHelpers.Combine(hashCode, Location?.GetHashCode() ?? 0);
return hashCode;
}
}
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics.Hashing;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// Provides an immutable list implementation which implements sequence equality.
Expand DownExpand Up@@ -72,15 +73,9 @@ public bool MoveNext()
}
}

public static class ImmutableEquatableArray
internal static class ImmutableEquatableArray
Comment thread
layomia marked this conversation as resolved.
Outdated
{
public static ImmutableEquatableArray<T> Empty<T>() where T : IEquatable<T>
=> ImmutableEquatableArray<T>.Empty;

public static ImmutableEquatableArray<T> ToImmutableEquatableArray<T>(this IEnumerable<T> values) where T : IEquatable<T>
=> new(values);

public static ImmutableEquatableArray<T> Create<T>(params T[] values) where T : IEquatable<T>
=> values is { Length: > 0 } ? new(values) : ImmutableEquatableArray<T>.Empty;
}
}
4 changes: 4 additions & 0 deletions src/libraries/Common/src/SourceGenerators/TypeModelHelper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,8 @@

using Microsoft.CodeAnalysis;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace SourceGenerators
{
Expand DownExpand Up@@ -32,5 +34,7 @@ void TraverseContainingTypes(INamedTypeSymbol current)
}
}
}

public static string GetFullyQualifiedName(this ITypeSymbol type) => type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
}
}
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using Microsoft.CodeAnalysis;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// An equatable value representing type identity.
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Xunit;

namespace SourceGenerators.Tests
{
internal static class GeneratorTestHelpers
{
/// <summary>
/// Asserts for structural equality, returning a path to the mismatching data when not equal.
/// </summary>
public static void AssertStructurallyEqual<T>(T expected, T actual)
{
CheckAreEqualCore(expected, actual, new());
static void CheckAreEqualCore(object expected, object actual, Stack<string> path)
{
if (expected is null || actual is null)
{
if (expected is not null || actual is not null)
{
FailNotEqual();
}

return;
}

Type type = expected.GetType();
if (type != actual.GetType())
{
FailNotEqual();
return;
}

if (expected is IEnumerable leftCollection)
{
if (actual is not IEnumerable rightCollection)
{
FailNotEqual();
return;
}

object?[] expectedValues = leftCollection.Cast<object?>().ToArray();
object?[] actualValues = rightCollection.Cast<object?>().ToArray();

for (int i = 0; i < Math.Max(expectedValues.Length, actualValues.Length); i++)
{
object? expectedElement = i < expectedValues.Length ? expectedValues[i] : "<end of collection>";
object? actualElement = i < actualValues.Length ? actualValues[i] : "<end of collection>";

path.Push($"[{i}]");
CheckAreEqualCore(expectedElement, actualElement, path);
path.Pop();
}
}

if (type.GetProperty("EqualityContract", BindingFlags.Instance | BindingFlags.NonPublic, null, returnType: typeof(Type), types: Array.Empty<Type>(), null) != null)
{
// Type is a C# record, run pointwise equality comparison.
foreach (PropertyInfo property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
path.Push("." + property.Name);
CheckAreEqualCore(property.GetValue(expected), property.GetValue(actual), path);
path.Pop();
}

return;
}

if (!expected.Equals(actual))
{
FailNotEqual();
}

void FailNotEqual() => Assert.Fail($"Value not equal in ${string.Join("", path.Reverse())}: expected {expected}, but was {actual}.");
}
}
}
}
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using SourceGenerators;

Expand All@@ -11,19 +10,22 @@ public sealed partial class ConfigurationBindingGenerator : IIncrementalGenerato
{
private sealed partial class Emitter
{
private readonly SourceProductionContext _context;
private readonly SourceGenerationSpec _sourceGenSpec;
private readonly InterceptorInfo _interceptorInfo;
private readonly BindingHelperInfo _bindingHelperInfo;
private readonly TypeIndex _typeIndex;

private readonly SourceWriter _writer = new();

public Emitter(SourceProductionContext context, SourceGenerationSpec sourceGenSpec)
public Emitter(SourceGenerationSpec sourceGenSpec)
{
_context = context;
_sourceGenSpec = sourceGenSpec;
_interceptorInfo = sourceGenSpec.InterceptorInfo;
_bindingHelperInfo = sourceGenSpec.BindingHelperInfo;
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
}

public void Emit()
public void Emit(SourceProductionContext context)
{
if (!ShouldEmitBindingExtensions())
if (!ShouldEmitMethods(MethodsToGen.Any))
{
return;
}
Expand DownExpand Up@@ -52,7 +54,7 @@ file static class {{Identifier.BindingExtensions}}

EmitEndBlock(); // Binding namespace.

_context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
}

private void EmitInterceptsLocationAttrDecl()
Expand All@@ -79,7 +81,7 @@ public InterceptsLocationAttribute(string filePath, int line, int column)

private void EmitUsingStatements()
{
foreach (string @namespace in _sourceGenSpec.Namespaces.ToImmutableSortedSet())
foreach (string @namespace in _bindingHelperInfo.Namespaces)
{
_writer.WriteLine($"using {@namespace};");
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Make config binding gen incremental by layomia · Pull Request #89587 · dotnet/runtime · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using System.Numerics.Hashing;
using Microsoft.CodeAnalysis;

namespace SourceGenerators;

/// <summary>
/// Descriptor for diagnostic instances using structural equality comparison.
/// Provides a work-around for https://github.com/dotnet/roslyn/issues/68291.
/// </summary>
internal readonly struct DiagnosticInfo : IEquatable<DiagnosticInfo>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, I believe the regex generator defines such a struct as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance this seems like a type that would be defined as a record if arrays were naturally equitable. Why wasn't this just defined as a record with ImmutableEquatableArray<object?> wrapping MessageArgs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would force an intermediate allocation from object[] to ImmutableEquatableArray then back to an object[] when the Diagnostic gets materialized. I think it's small enough to justify a bespoke type. Longer term though we should be looking at addressing dotnet/roslyn#68291 and use Diagnostic in the equatable models directly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model. Location has a reference to the SyntaxTree that it was created from, and that will change every time the file changes. This wrapper has similar issues: even if message args compared correctly, it would still be broken for equality.

@layomialayomiaSep 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model.

Per #89587 (comment) this bit is mitigated for this PR, follow up in #92509.

{
public DiagnosticDescriptor Descriptor { get; private init; }
public object?[] MessageArgs { get; private init; }
public Location? Location { get; private init; }

public static DiagnosticInfo Create(DiagnosticDescriptor descriptor, Location? location, object?[]? messageArgs)
{
Location? trimmedLocation = location is null ? null : GetTrimmedLocation(location);

return new DiagnosticInfo
{
Descriptor = descriptor,
Location = trimmedLocation,
MessageArgs = messageArgs ?? Array.Empty<object?>()
};

// Creates a copy of the Location instance that does not capture a reference to Compilation.
static Location GetTrimmedLocation(Location location)
=> Location.Create(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span);
}

public Diagnostic CreateDiagnostic()
=> Diagnostic.Create(Descriptor, Location, MessageArgs);

public override readonly bool Equals(object? obj) => obj is DiagnosticInfo info && Equals(info);

public readonly bool Equals(DiagnosticInfo other)
{
return Descriptor.Equals(other.Descriptor) &&
MessageArgs.SequenceEqual(other.MessageArgs) &&
Location == other.Location;
}

public override readonly int GetHashCode()
{
int hashCode = Descriptor.GetHashCode();
foreach (object? messageArg in MessageArgs)
{
hashCode = HashHelpers.Combine(hashCode, messageArg?.GetHashCode() ?? 0);
}

hashCode = HashHelpers.Combine(hashCode, Location?.GetHashCode() ?? 0);
return hashCode;
}
}
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics.Hashing;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// Provides an immutable list implementation which implements sequence equality.
Expand DownExpand Up@@ -72,15 +73,9 @@ public bool MoveNext()
}
}

public static class ImmutableEquatableArray
internal static class ImmutableEquatableArray
Comment thread
layomia marked this conversation as resolved.
Outdated
{
public static ImmutableEquatableArray<T> Empty<T>() where T : IEquatable<T>
=> ImmutableEquatableArray<T>.Empty;

public static ImmutableEquatableArray<T> ToImmutableEquatableArray<T>(this IEnumerable<T> values) where T : IEquatable<T>
=> new(values);

public static ImmutableEquatableArray<T> Create<T>(params T[] values) where T : IEquatable<T>
=> values is { Length: > 0 } ? new(values) : ImmutableEquatableArray<T>.Empty;
}
}
4 changes: 4 additions & 0 deletions src/libraries/Common/src/SourceGenerators/TypeModelHelper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,8 @@

using Microsoft.CodeAnalysis;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace SourceGenerators
{
Expand DownExpand Up@@ -32,5 +34,7 @@ void TraverseContainingTypes(INamedTypeSymbol current)
}
}
}

public static string GetFullyQualifiedName(this ITypeSymbol type) => type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
}
}
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using Microsoft.CodeAnalysis;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// An equatable value representing type identity.
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Xunit;

namespace SourceGenerators.Tests
{
internal static class GeneratorTestHelpers
{
/// <summary>
/// Asserts for structural equality, returning a path to the mismatching data when not equal.
/// </summary>
public static void AssertStructurallyEqual<T>(T expected, T actual)
{
CheckAreEqualCore(expected, actual, new());
static void CheckAreEqualCore(object expected, object actual, Stack<string> path)
{
if (expected is null || actual is null)
{
if (expected is not null || actual is not null)
{
FailNotEqual();
}

return;
}

Type type = expected.GetType();
if (type != actual.GetType())
{
FailNotEqual();
return;
}

if (expected is IEnumerable leftCollection)
{
if (actual is not IEnumerable rightCollection)
{
FailNotEqual();
return;
}

object?[] expectedValues = leftCollection.Cast<object?>().ToArray();
object?[] actualValues = rightCollection.Cast<object?>().ToArray();

for (int i = 0; i < Math.Max(expectedValues.Length, actualValues.Length); i++)
{
object? expectedElement = i < expectedValues.Length ? expectedValues[i] : "<end of collection>";
object? actualElement = i < actualValues.Length ? actualValues[i] : "<end of collection>";

path.Push($"[{i}]");
CheckAreEqualCore(expectedElement, actualElement, path);
path.Pop();
}
}

if (type.GetProperty("EqualityContract", BindingFlags.Instance | BindingFlags.NonPublic, null, returnType: typeof(Type), types: Array.Empty<Type>(), null) != null)
{
// Type is a C# record, run pointwise equality comparison.
foreach (PropertyInfo property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
path.Push("." + property.Name);
CheckAreEqualCore(property.GetValue(expected), property.GetValue(actual), path);
path.Pop();
}

return;
}

if (!expected.Equals(actual))
{
FailNotEqual();
}

void FailNotEqual() => Assert.Fail($"Value not equal in ${string.Join("", path.Reverse())}: expected {expected}, but was {actual}.");
}
}
}
}
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using SourceGenerators;

Expand All@@ -11,19 +10,22 @@ public sealed partial class ConfigurationBindingGenerator : IIncrementalGenerato
{
private sealed partial class Emitter
{
private readonly SourceProductionContext _context;
private readonly SourceGenerationSpec _sourceGenSpec;
private readonly InterceptorInfo _interceptorInfo;
private readonly BindingHelperInfo _bindingHelperInfo;
private readonly TypeIndex _typeIndex;

private readonly SourceWriter _writer = new();

public Emitter(SourceProductionContext context, SourceGenerationSpec sourceGenSpec)
public Emitter(SourceGenerationSpec sourceGenSpec)
{
_context = context;
_sourceGenSpec = sourceGenSpec;
_interceptorInfo = sourceGenSpec.InterceptorInfo;
_bindingHelperInfo = sourceGenSpec.BindingHelperInfo;
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
}

public void Emit()
public void Emit(SourceProductionContext context)
{
if (!ShouldEmitBindingExtensions())
if (!ShouldEmitMethods(MethodsToGen.Any))
{
return;
}
Expand DownExpand Up@@ -52,7 +54,7 @@ file static class {{Identifier.BindingExtensions}}

EmitEndBlock(); // Binding namespace.

_context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
}

private void EmitInterceptsLocationAttrDecl()
Expand All@@ -79,7 +81,7 @@ public InterceptsLocationAttribute(string filePath, int line, int column)

private void EmitUsingStatements()
{
foreach (string @namespace in _sourceGenSpec.Namespaces.ToImmutableSortedSet())
foreach (string @namespace in _bindingHelperInfo.Namespaces)
{
_writer.WriteLine($"using {@namespace};");
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Make config binding gen incremental by layomia · Pull Request #89587 · dotnet/runtime · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using System.Numerics.Hashing;
using Microsoft.CodeAnalysis;

namespace SourceGenerators;

/// <summary>
/// Descriptor for diagnostic instances using structural equality comparison.
/// Provides a work-around for https://github.com/dotnet/roslyn/issues/68291.
/// </summary>
internal readonly struct DiagnosticInfo : IEquatable<DiagnosticInfo>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, I believe the regex generator defines such a struct as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance this seems like a type that would be defined as a record if arrays were naturally equitable. Why wasn't this just defined as a record with ImmutableEquatableArray<object?> wrapping MessageArgs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would force an intermediate allocation from object[] to ImmutableEquatableArray then back to an object[] when the Diagnostic gets materialized. I think it's small enough to justify a bespoke type. Longer term though we should be looking at addressing dotnet/roslyn#68291 and use Diagnostic in the equatable models directly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model. Location has a reference to the SyntaxTree that it was created from, and that will change every time the file changes. This wrapper has similar issues: even if message args compared correctly, it would still be broken for equality.

@layomialayomiaSep 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model.

Per #89587 (comment) this bit is mitigated for this PR, follow up in #92509.

{
public DiagnosticDescriptor Descriptor { get; private init; }
public object?[] MessageArgs { get; private init; }
public Location? Location { get; private init; }

public static DiagnosticInfo Create(DiagnosticDescriptor descriptor, Location? location, object?[]? messageArgs)
{
Location? trimmedLocation = location is null ? null : GetTrimmedLocation(location);

return new DiagnosticInfo
{
Descriptor = descriptor,
Location = trimmedLocation,
MessageArgs = messageArgs ?? Array.Empty<object?>()
};

// Creates a copy of the Location instance that does not capture a reference to Compilation.
static Location GetTrimmedLocation(Location location)
=> Location.Create(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span);
}

public Diagnostic CreateDiagnostic()
=> Diagnostic.Create(Descriptor, Location, MessageArgs);

public override readonly bool Equals(object? obj) => obj is DiagnosticInfo info && Equals(info);

public readonly bool Equals(DiagnosticInfo other)
{
return Descriptor.Equals(other.Descriptor) &&
MessageArgs.SequenceEqual(other.MessageArgs) &&
Location == other.Location;
}

public override readonly int GetHashCode()
{
int hashCode = Descriptor.GetHashCode();
foreach (object? messageArg in MessageArgs)
{
hashCode = HashHelpers.Combine(hashCode, messageArg?.GetHashCode() ?? 0);
}

hashCode = HashHelpers.Combine(hashCode, Location?.GetHashCode() ?? 0);
return hashCode;
}
}
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics.Hashing;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// Provides an immutable list implementation which implements sequence equality.
Expand DownExpand Up@@ -72,15 +73,9 @@ public bool MoveNext()
}
}

public static class ImmutableEquatableArray
internal static class ImmutableEquatableArray
Comment thread
layomia marked this conversation as resolved.
Outdated
{
public static ImmutableEquatableArray<T> Empty<T>() where T : IEquatable<T>
=> ImmutableEquatableArray<T>.Empty;

public static ImmutableEquatableArray<T> ToImmutableEquatableArray<T>(this IEnumerable<T> values) where T : IEquatable<T>
=> new(values);

public static ImmutableEquatableArray<T> Create<T>(params T[] values) where T : IEquatable<T>
=> values is { Length: > 0 } ? new(values) : ImmutableEquatableArray<T>.Empty;
}
}
4 changes: 4 additions & 0 deletions src/libraries/Common/src/SourceGenerators/TypeModelHelper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,8 @@

using Microsoft.CodeAnalysis;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace SourceGenerators
{
Expand DownExpand Up@@ -32,5 +34,7 @@ void TraverseContainingTypes(INamedTypeSymbol current)
}
}
}

public static string GetFullyQualifiedName(this ITypeSymbol type) => type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
}
}
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using Microsoft.CodeAnalysis;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// An equatable value representing type identity.
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Xunit;

namespace SourceGenerators.Tests
{
internal static class GeneratorTestHelpers
{
/// <summary>
/// Asserts for structural equality, returning a path to the mismatching data when not equal.
/// </summary>
public static void AssertStructurallyEqual<T>(T expected, T actual)
{
CheckAreEqualCore(expected, actual, new());
static void CheckAreEqualCore(object expected, object actual, Stack<string> path)
{
if (expected is null || actual is null)
{
if (expected is not null || actual is not null)
{
FailNotEqual();
}

return;
}

Type type = expected.GetType();
if (type != actual.GetType())
{
FailNotEqual();
return;
}

if (expected is IEnumerable leftCollection)
{
if (actual is not IEnumerable rightCollection)
{
FailNotEqual();
return;
}

object?[] expectedValues = leftCollection.Cast<object?>().ToArray();
object?[] actualValues = rightCollection.Cast<object?>().ToArray();

for (int i = 0; i < Math.Max(expectedValues.Length, actualValues.Length); i++)
{
object? expectedElement = i < expectedValues.Length ? expectedValues[i] : "<end of collection>";
object? actualElement = i < actualValues.Length ? actualValues[i] : "<end of collection>";

path.Push($"[{i}]");
CheckAreEqualCore(expectedElement, actualElement, path);
path.Pop();
}
}

if (type.GetProperty("EqualityContract", BindingFlags.Instance | BindingFlags.NonPublic, null, returnType: typeof(Type), types: Array.Empty<Type>(), null) != null)
{
// Type is a C# record, run pointwise equality comparison.
foreach (PropertyInfo property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
path.Push("." + property.Name);
CheckAreEqualCore(property.GetValue(expected), property.GetValue(actual), path);
path.Pop();
}

return;
}

if (!expected.Equals(actual))
{
FailNotEqual();
}

void FailNotEqual() => Assert.Fail($"Value not equal in ${string.Join("", path.Reverse())}: expected {expected}, but was {actual}.");
}
}
}
}
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using SourceGenerators;

Expand All@@ -11,19 +10,22 @@ public sealed partial class ConfigurationBindingGenerator : IIncrementalGenerato
{
private sealed partial class Emitter
{
private readonly SourceProductionContext _context;
private readonly SourceGenerationSpec _sourceGenSpec;
private readonly InterceptorInfo _interceptorInfo;
private readonly BindingHelperInfo _bindingHelperInfo;
private readonly TypeIndex _typeIndex;

private readonly SourceWriter _writer = new();

public Emitter(SourceProductionContext context, SourceGenerationSpec sourceGenSpec)
public Emitter(SourceGenerationSpec sourceGenSpec)
{
_context = context;
_sourceGenSpec = sourceGenSpec;
_interceptorInfo = sourceGenSpec.InterceptorInfo;
_bindingHelperInfo = sourceGenSpec.BindingHelperInfo;
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
}

public void Emit()
public void Emit(SourceProductionContext context)
{
if (!ShouldEmitBindingExtensions())
if (!ShouldEmitMethods(MethodsToGen.Any))
{
return;
}
Expand DownExpand Up@@ -52,7 +54,7 @@ file static class {{Identifier.BindingExtensions}}

EmitEndBlock(); // Binding namespace.

_context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
}

private void EmitInterceptsLocationAttrDecl()
Expand All@@ -79,7 +81,7 @@ public InterceptsLocationAttribute(string filePath, int line, int column)

private void EmitUsingStatements()
{
foreach (string @namespace in _sourceGenSpec.Namespaces.ToImmutableSortedSet())
foreach (string @namespace in _bindingHelperInfo.Namespaces)
{
_writer.WriteLine($"using {@namespace};");
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Make config binding gen incremental by layomia · Pull Request #89587 · dotnet/runtime · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using System.Numerics.Hashing;
using Microsoft.CodeAnalysis;

namespace SourceGenerators;

/// <summary>
/// Descriptor for diagnostic instances using structural equality comparison.
/// Provides a work-around for https://github.com/dotnet/roslyn/issues/68291.
/// </summary>
internal readonly struct DiagnosticInfo : IEquatable<DiagnosticInfo>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, I believe the regex generator defines such a struct as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance this seems like a type that would be defined as a record if arrays were naturally equitable. Why wasn't this just defined as a record with ImmutableEquatableArray<object?> wrapping MessageArgs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would force an intermediate allocation from object[] to ImmutableEquatableArray then back to an object[] when the Diagnostic gets materialized. I think it's small enough to justify a bespoke type. Longer term though we should be looking at addressing dotnet/roslyn#68291 and use Diagnostic in the equatable models directly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model. Location has a reference to the SyntaxTree that it was created from, and that will change every time the file changes. This wrapper has similar issues: even if message args compared correctly, it would still be broken for equality.

@layomialayomiaSep 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model.

Per #89587 (comment) this bit is mitigated for this PR, follow up in #92509.

{
public DiagnosticDescriptor Descriptor { get; private init; }
public object?[] MessageArgs { get; private init; }
public Location? Location { get; private init; }

public static DiagnosticInfo Create(DiagnosticDescriptor descriptor, Location? location, object?[]? messageArgs)
{
Location? trimmedLocation = location is null ? null : GetTrimmedLocation(location);

return new DiagnosticInfo
{
Descriptor = descriptor,
Location = trimmedLocation,
MessageArgs = messageArgs ?? Array.Empty<object?>()
};

// Creates a copy of the Location instance that does not capture a reference to Compilation.
static Location GetTrimmedLocation(Location location)
=> Location.Create(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span);
}

public Diagnostic CreateDiagnostic()
=> Diagnostic.Create(Descriptor, Location, MessageArgs);

public override readonly bool Equals(object? obj) => obj is DiagnosticInfo info && Equals(info);

public readonly bool Equals(DiagnosticInfo other)
{
return Descriptor.Equals(other.Descriptor) &&
MessageArgs.SequenceEqual(other.MessageArgs) &&
Location == other.Location;
}

public override readonly int GetHashCode()
{
int hashCode = Descriptor.GetHashCode();
foreach (object? messageArg in MessageArgs)
{
hashCode = HashHelpers.Combine(hashCode, messageArg?.GetHashCode() ?? 0);
}

hashCode = HashHelpers.Combine(hashCode, Location?.GetHashCode() ?? 0);
return hashCode;
}
}
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics.Hashing;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// Provides an immutable list implementation which implements sequence equality.
Expand DownExpand Up@@ -72,15 +73,9 @@ public bool MoveNext()
}
}

public static class ImmutableEquatableArray
internal static class ImmutableEquatableArray
Comment thread
layomia marked this conversation as resolved.
Outdated
{
public static ImmutableEquatableArray<T> Empty<T>() where T : IEquatable<T>
=> ImmutableEquatableArray<T>.Empty;

public static ImmutableEquatableArray<T> ToImmutableEquatableArray<T>(this IEnumerable<T> values) where T : IEquatable<T>
=> new(values);

public static ImmutableEquatableArray<T> Create<T>(params T[] values) where T : IEquatable<T>
=> values is { Length: > 0 } ? new(values) : ImmutableEquatableArray<T>.Empty;
}
}
4 changes: 4 additions & 0 deletions src/libraries/Common/src/SourceGenerators/TypeModelHelper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,8 @@

using Microsoft.CodeAnalysis;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace SourceGenerators
{
Expand DownExpand Up@@ -32,5 +34,7 @@ void TraverseContainingTypes(INamedTypeSymbol current)
}
}
}

public static string GetFullyQualifiedName(this ITypeSymbol type) => type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
}
}
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using Microsoft.CodeAnalysis;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// An equatable value representing type identity.
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Xunit;

namespace SourceGenerators.Tests
{
internal static class GeneratorTestHelpers
{
/// <summary>
/// Asserts for structural equality, returning a path to the mismatching data when not equal.
/// </summary>
public static void AssertStructurallyEqual<T>(T expected, T actual)
{
CheckAreEqualCore(expected, actual, new());
static void CheckAreEqualCore(object expected, object actual, Stack<string> path)
{
if (expected is null || actual is null)
{
if (expected is not null || actual is not null)
{
FailNotEqual();
}

return;
}

Type type = expected.GetType();
if (type != actual.GetType())
{
FailNotEqual();
return;
}

if (expected is IEnumerable leftCollection)
{
if (actual is not IEnumerable rightCollection)
{
FailNotEqual();
return;
}

object?[] expectedValues = leftCollection.Cast<object?>().ToArray();
object?[] actualValues = rightCollection.Cast<object?>().ToArray();

for (int i = 0; i < Math.Max(expectedValues.Length, actualValues.Length); i++)
{
object? expectedElement = i < expectedValues.Length ? expectedValues[i] : "<end of collection>";
object? actualElement = i < actualValues.Length ? actualValues[i] : "<end of collection>";

path.Push($"[{i}]");
CheckAreEqualCore(expectedElement, actualElement, path);
path.Pop();
}
}

if (type.GetProperty("EqualityContract", BindingFlags.Instance | BindingFlags.NonPublic, null, returnType: typeof(Type), types: Array.Empty<Type>(), null) != null)
{
// Type is a C# record, run pointwise equality comparison.
foreach (PropertyInfo property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
path.Push("." + property.Name);
CheckAreEqualCore(property.GetValue(expected), property.GetValue(actual), path);
path.Pop();
}

return;
}

if (!expected.Equals(actual))
{
FailNotEqual();
}

void FailNotEqual() => Assert.Fail($"Value not equal in ${string.Join("", path.Reverse())}: expected {expected}, but was {actual}.");
}
}
}
}
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using SourceGenerators;

Expand All@@ -11,19 +10,22 @@ public sealed partial class ConfigurationBindingGenerator : IIncrementalGenerato
{
private sealed partial class Emitter
{
private readonly SourceProductionContext _context;
private readonly SourceGenerationSpec _sourceGenSpec;
private readonly InterceptorInfo _interceptorInfo;
private readonly BindingHelperInfo _bindingHelperInfo;
private readonly TypeIndex _typeIndex;

private readonly SourceWriter _writer = new();

public Emitter(SourceProductionContext context, SourceGenerationSpec sourceGenSpec)
public Emitter(SourceGenerationSpec sourceGenSpec)
{
_context = context;
_sourceGenSpec = sourceGenSpec;
_interceptorInfo = sourceGenSpec.InterceptorInfo;
_bindingHelperInfo = sourceGenSpec.BindingHelperInfo;
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
}

public void Emit()
public void Emit(SourceProductionContext context)
{
if (!ShouldEmitBindingExtensions())
if (!ShouldEmitMethods(MethodsToGen.Any))
{
return;
}
Expand DownExpand Up@@ -52,7 +54,7 @@ file static class {{Identifier.BindingExtensions}}

EmitEndBlock(); // Binding namespace.

_context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
}

private void EmitInterceptsLocationAttrDecl()
Expand All@@ -79,7 +81,7 @@ public InterceptsLocationAttribute(string filePath, int line, int column)

private void EmitUsingStatements()
{
foreach (string @namespace in _sourceGenSpec.Namespaces.ToImmutableSortedSet())
foreach (string @namespace in _bindingHelperInfo.Namespaces)
{
_writer.WriteLine($"using {@namespace};");
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Make config binding gen incremental by layomia · Pull Request #89587 · dotnet/runtime · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using System.Numerics.Hashing;
using Microsoft.CodeAnalysis;

namespace SourceGenerators;

/// <summary>
/// Descriptor for diagnostic instances using structural equality comparison.
/// Provides a work-around for https://github.com/dotnet/roslyn/issues/68291.
/// </summary>
internal readonly struct DiagnosticInfo : IEquatable<DiagnosticInfo>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, I believe the regex generator defines such a struct as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance this seems like a type that would be defined as a record if arrays were naturally equitable. Why wasn't this just defined as a record with ImmutableEquatableArray<object?> wrapping MessageArgs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would force an intermediate allocation from object[] to ImmutableEquatableArray then back to an object[] when the Diagnostic gets materialized. I think it's small enough to justify a bespoke type. Longer term though we should be looking at addressing dotnet/roslyn#68291 and use Diagnostic in the equatable models directly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model. Location has a reference to the SyntaxTree that it was created from, and that will change every time the file changes. This wrapper has similar issues: even if message args compared correctly, it would still be broken for equality.

@layomialayomiaSep 22, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostic will never be ok to use in a model.

Per #89587 (comment) this bit is mitigated for this PR, follow up in #92509.

{
public DiagnosticDescriptor Descriptor { get; private init; }
public object?[] MessageArgs { get; private init; }
public Location? Location { get; private init; }

public static DiagnosticInfo Create(DiagnosticDescriptor descriptor, Location? location, object?[]? messageArgs)
{
Location? trimmedLocation = location is null ? null : GetTrimmedLocation(location);

return new DiagnosticInfo
{
Descriptor = descriptor,
Location = trimmedLocation,
MessageArgs = messageArgs ?? Array.Empty<object?>()
};

// Creates a copy of the Location instance that does not capture a reference to Compilation.
static Location GetTrimmedLocation(Location location)
=> Location.Create(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span);
}

public Diagnostic CreateDiagnostic()
=> Diagnostic.Create(Descriptor, Location, MessageArgs);

public override readonly bool Equals(object? obj) => obj is DiagnosticInfo info && Equals(info);

public readonly bool Equals(DiagnosticInfo other)
{
return Descriptor.Equals(other.Descriptor) &&
MessageArgs.SequenceEqual(other.MessageArgs) &&
Location == other.Location;
}

public override readonly int GetHashCode()
{
int hashCode = Descriptor.GetHashCode();
foreach (object? messageArg in MessageArgs)
{
hashCode = HashHelpers.Combine(hashCode, messageArg?.GetHashCode() ?? 0);
}

hashCode = HashHelpers.Combine(hashCode, Location?.GetHashCode() ?? 0);
return hashCode;
}
}
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Numerics.Hashing;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// Provides an immutable list implementation which implements sequence equality.
Expand DownExpand Up@@ -72,15 +73,9 @@ public bool MoveNext()
}
}

public static class ImmutableEquatableArray
internal static class ImmutableEquatableArray
Comment thread
layomia marked this conversation as resolved.
Outdated
{
public static ImmutableEquatableArray<T> Empty<T>() where T : IEquatable<T>
=> ImmutableEquatableArray<T>.Empty;

public static ImmutableEquatableArray<T> ToImmutableEquatableArray<T>(this IEnumerable<T> values) where T : IEquatable<T>
=> new(values);

public static ImmutableEquatableArray<T> Create<T>(params T[] values) where T : IEquatable<T>
=> values is { Length: > 0 } ? new(values) : ImmutableEquatableArray<T>.Empty;
}
}
4 changes: 4 additions & 0 deletions src/libraries/Common/src/SourceGenerators/TypeModelHelper.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,8 @@

using Microsoft.CodeAnalysis;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace SourceGenerators
{
Expand DownExpand Up@@ -32,5 +34,7 @@ void TraverseContainingTypes(INamedTypeSymbol current)
}
}
}

public static string GetFullyQualifiedName(this ITypeSymbol type) => type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
}
}
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using Microsoft.CodeAnalysis;

namespace System.Text.Json.SourceGeneration
namespace SourceGenerators
{
/// <summary>
/// An equatable value representing type identity.
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Xunit;

namespace SourceGenerators.Tests
{
internal static class GeneratorTestHelpers
{
/// <summary>
/// Asserts for structural equality, returning a path to the mismatching data when not equal.
/// </summary>
public static void AssertStructurallyEqual<T>(T expected, T actual)
{
CheckAreEqualCore(expected, actual, new());
static void CheckAreEqualCore(object expected, object actual, Stack<string> path)
{
if (expected is null || actual is null)
{
if (expected is not null || actual is not null)
{
FailNotEqual();
}

return;
}

Type type = expected.GetType();
if (type != actual.GetType())
{
FailNotEqual();
return;
}

if (expected is IEnumerable leftCollection)
{
if (actual is not IEnumerable rightCollection)
{
FailNotEqual();
return;
}

object?[] expectedValues = leftCollection.Cast<object?>().ToArray();
object?[] actualValues = rightCollection.Cast<object?>().ToArray();

for (int i = 0; i < Math.Max(expectedValues.Length, actualValues.Length); i++)
{
object? expectedElement = i < expectedValues.Length ? expectedValues[i] : "<end of collection>";
object? actualElement = i < actualValues.Length ? actualValues[i] : "<end of collection>";

path.Push($"[{i}]");
CheckAreEqualCore(expectedElement, actualElement, path);
path.Pop();
}
}

if (type.GetProperty("EqualityContract", BindingFlags.Instance | BindingFlags.NonPublic, null, returnType: typeof(Type), types: Array.Empty<Type>(), null) != null)
{
// Type is a C# record, run pointwise equality comparison.
foreach (PropertyInfo property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
path.Push("." + property.Name);
CheckAreEqualCore(property.GetValue(expected), property.GetValue(actual), path);
path.Pop();
}

return;
}

if (!expected.Equals(actual))
{
FailNotEqual();
}

void FailNotEqual() => Assert.Fail($"Value not equal in ${string.Join("", path.Reverse())}: expected {expected}, but was {actual}.");
}
}
}
}
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using SourceGenerators;

Expand All@@ -11,19 +10,22 @@ public sealed partial class ConfigurationBindingGenerator : IIncrementalGenerato
{
private sealed partial class Emitter
{
private readonly SourceProductionContext _context;
private readonly SourceGenerationSpec _sourceGenSpec;
private readonly InterceptorInfo _interceptorInfo;
private readonly BindingHelperInfo _bindingHelperInfo;
private readonly TypeIndex _typeIndex;

private readonly SourceWriter _writer = new();

public Emitter(SourceProductionContext context, SourceGenerationSpec sourceGenSpec)
public Emitter(SourceGenerationSpec sourceGenSpec)
{
_context = context;
_sourceGenSpec = sourceGenSpec;
_interceptorInfo = sourceGenSpec.InterceptorInfo;
_bindingHelperInfo = sourceGenSpec.BindingHelperInfo;
_typeIndex = new TypeIndex(sourceGenSpec.ConfigTypes);
}

public void Emit()
public void Emit(SourceProductionContext context)
{
if (!ShouldEmitBindingExtensions())
if (!ShouldEmitMethods(MethodsToGen.Any))
{
return;
}
Expand DownExpand Up@@ -52,7 +54,7 @@ file static class {{Identifier.BindingExtensions}}

EmitEndBlock(); // Binding namespace.

_context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
context.AddSource($"{Identifier.BindingExtensions}.g.cs", _writer.ToSourceText());
}

private void EmitInterceptsLocationAttrDecl()
Expand All@@ -79,7 +81,7 @@ public InterceptsLocationAttribute(string filePath, int line, int column)

private void EmitUsingStatements()
{
foreach (string @namespace in _sourceGenSpec.Namespaces.ToImmutableSortedSet())
foreach (string @namespace in _bindingHelperInfo.Namespaces)
{
_writer.WriteLine($"using {@namespace};");
}
Expand Down
Loading