Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove syntax serialization logic. #70277

Merged
merged 31 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
cb0102b
Remove code for node serialization
CyrusNajmabadi Oct 6, 2023
6fb6240
Update generators
CyrusNajmabadi Oct 6, 2023
8839f7f
Fix
CyrusNajmabadi Oct 6, 2023
06a1674
Remove recursion from object serialization
CyrusNajmabadi Oct 6, 2023
bbbb772
Remove more
CyrusNajmabadi Oct 6, 2023
377e4cf
in progress
CyrusNajmabadi Oct 6, 2023
8c3fe33
Remove IObjectWritable
CyrusNajmabadi Oct 6, 2023
ab73f4d
Remove tests
CyrusNajmabadi Oct 6, 2023
265799a
Remove tests
CyrusNajmabadi Oct 6, 2023
77df906
Merge remote-tracking branch 'upstream/main' into nodeSerialization2
CyrusNajmabadi Oct 25, 2023
9dcb386
Merge remote-tracking branch 'upstream/main' into nodeSerialization2
CyrusNajmabadi Oct 26, 2023
7f9c2b9
fix test
CyrusNajmabadi Oct 26, 2023
8375ba6
Merge branch 'DisableTests' into nodeSerialization2
CyrusNajmabadi Oct 26, 2023
b76ae85
Merge remote-tracking branch 'upstream/main' into nodeSerialization2
CyrusNajmabadi Oct 27, 2023
66bb7ea
Obsolete error
CyrusNajmabadi Oct 27, 2023
8af6459
REmove stale benchmark
CyrusNajmabadi Oct 27, 2023
26a3886
Merge remote-tracking branch 'genlu/SKiptest' into nodeSerialization2
CyrusNajmabadi Oct 27, 2023
df13aa3
Merge remote-tracking branch 'upstream/main' into nodeSerialization2
CyrusNajmabadi Nov 6, 2023
928db0d
remove unused methods
CyrusNajmabadi Nov 6, 2023
cc33b1d
Fix comment
CyrusNajmabadi Nov 6, 2023
256e4d4
Update breaking changes docs
CyrusNajmabadi Nov 6, 2023
e095de0
Remove recursion depth checks
CyrusNajmabadi Nov 6, 2023
a325c49
Remove unused type
CyrusNajmabadi Nov 6, 2023
05638e1
remove unneeded logic
CyrusNajmabadi Nov 6, 2023
3aea819
Move serialization code entirely out of hte compiler layer. it now e…
CyrusNajmabadi Nov 6, 2023
9f05fa7
Simplify
CyrusNajmabadi Nov 6, 2023
a7af896
Simplify
CyrusNajmabadi Nov 6, 2023
71e36d1
Update src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Seri…
CyrusNajmabadi Nov 6, 2023
fb5acb1
Lint
CyrusNajmabadi Nov 6, 2023
137d60d
Merge branch 'nodeSerialization2' of https://github.com/CyrusNajmabad…
CyrusNajmabadi Nov 6, 2023
59dd654
Lint
CyrusNajmabadi Nov 6, 2023
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
10 changes: 9 additions & 1 deletion docs/Breaking API Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,12 @@ This could already happen in other load scenarios but this change moves it into

The ability to serialize/deserialize a SyntaxNode to/from a Stream has been deprecated. The code for this still exists in Roslyn, but attempting to call the APIs to perform these functions will result in 'Obsolete' warnings being reported. A future version of Roslyn will remove this functionality entirely. This functionality could only work for a host that wrote out the nodes to a stream, and later read it back in within the same process instance. It could not be used to communicate across processes, or for persisting nodes to disk to be read in at a later time by a new host sessions. This functionality originally existed for the days when Roslyn was hosted in 32bit processes with limited address space. That is no longer a mainline supported scenario. Clients can get similar functionality by persisting the text of the node, and parsing it back out when needed.

PR: https://github.com/dotnet/roslyn/pull/70365
PR: https://github.com/dotnet/roslyn/pull/70365

# Version 4.9.0

### Obsoletion and removal of SyntaxNode serialization.

Continuation of the deprecation that happened in 4.8.0 (see information above). In 4.9.0 this functionality is now entirely removed, and will issue both an obsoletion error, and will throw at runtime if the APIs are used.

PR: https://github.com/dotnet/roslyn/pull/70277
39 changes: 1 addition & 38 deletions src/Analyzers/Core/Analyzers/Helpers/DiagnosticHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -276,14 +276,11 @@ public static Diagnostic CreateWithMessage(
return $"https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/{id.ToLowerInvariant()}";
}

public sealed class LocalizableStringWithArguments : LocalizableString, IObjectWritable
public sealed class LocalizableStringWithArguments : LocalizableString
Copy link
Member Author

Choose a reason for hiding this comment

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

IObjectWritable as a concept goes away. Instead of the serialization system taking in arbitrary objects, querying for that, and having that object then decide what to do, serialization is owned entirely on the outside by the objects, who just use our serialization system as a dumb stream wrapper to do things like write ints, bools, and strings to. Only basic constructs are supported (like primitive types, and arrays of types). Arbitrary object graphs are now handled by the caller (note: we have no such cases in roslyn anyways. it was only 'syntax' with its heterogenous graph structure).

Copy link
Member Author

Choose a reason for hiding this comment

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

LocalizableStirng used to have to be serializable, because it was stored in diagnostics as the messages, and diagnostics were stored inside a tree for syntax errors, and as such needed to be serializable if we serialized the tree. Now that we no longer serialize trees, neither diagnostics nor locstrings need this functionality.

{
private readonly LocalizableString _messageFormat;
private readonly string[] _formatArguments;

static LocalizableStringWithArguments()
=> ObjectBinder.RegisterTypeReader(typeof(LocalizableStringWithArguments), reader => new LocalizableStringWithArguments(reader));
Copy link
Member Author

Choose a reason for hiding this comment

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

ObjectBinder.RegisterTypeReader was how types specified how they could be deseriealized. But it worked by registering a callback function at an index in an array, then emitting that array index into the serialized stream of bytes. this, of course, coudl not work across processes or across sessions as that index woudl never be the same. this is why this functionality is so practically useless.


public LocalizableStringWithArguments(LocalizableString messageFormat, params object[] formatArguments)
{
if (messageFormat == null)
Expand All @@ -304,40 +301,6 @@ public LocalizableStringWithArguments(LocalizableString messageFormat, params ob
}
}

private LocalizableStringWithArguments(ObjectReader reader)
{
_messageFormat = (LocalizableString)reader.ReadValue();

var length = reader.ReadInt32();
if (length == 0)
{
_formatArguments = Array.Empty<string>();
}
else
{
using var _ = ArrayBuilder<string>.GetInstance(length, out var argumentsBuilder);
for (var i = 0; i < length; i++)
{
argumentsBuilder.Add(reader.ReadString());
}

_formatArguments = argumentsBuilder.ToArray();
}
}

bool IObjectWritable.ShouldReuseInSerialization => false;

void IObjectWritable.WriteTo(ObjectWriter writer)
{
writer.WriteValue(_messageFormat);
var length = _formatArguments.Length;
writer.WriteInt32(length);
for (var i = 0; i < length; i++)
{
writer.WriteString(_formatArguments[i]);
}
}

protected override string GetText(IFormatProvider? formatProvider)
{
var messageFormat = _messageFormat.ToString(formatProvider);
Expand Down
14 changes: 1 addition & 13 deletions src/Compilers/CSharp/Portable/Errors/MessageProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,14 @@

namespace Microsoft.CodeAnalysis.CSharp
{
internal sealed class MessageProvider : CommonMessageProvider, IObjectWritable
internal sealed class MessageProvider : CommonMessageProvider
Copy link
Member Author

Choose a reason for hiding this comment

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

MessageProvider is part of loc strings. Also doesn't need serialization now.

{
public static readonly MessageProvider Instance = new MessageProvider();

static MessageProvider()
{
ObjectBinder.RegisterTypeReader(typeof(MessageProvider), r => Instance);
}

private MessageProvider()
{
}

bool IObjectWritable.ShouldReuseInSerialization => true;

void IObjectWritable.WriteTo(ObjectWriter writer)
{
// write nothing, always read/deserialized as global Instance
}

public override DiagnosticSeverity GetSeverity(int code)
{
return ErrorFacts.GetSeverity((ErrorCode)code);
Expand Down
23 changes: 0 additions & 23 deletions src/Compilers/CSharp/Portable/Errors/SyntaxDiagnosticInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@ namespace Microsoft.CodeAnalysis.CSharp
{
internal class SyntaxDiagnosticInfo : DiagnosticInfo
Copy link
Member Author

Choose a reason for hiding this comment

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

diagnostic-infos are part of diagnostics. which no longer need to be serialized now.

{
static SyntaxDiagnosticInfo()
{
ObjectBinder.RegisterTypeReader(typeof(SyntaxDiagnosticInfo), r => new SyntaxDiagnosticInfo(r));
}

internal readonly int Offset;
internal readonly int Width;

Expand Down Expand Up @@ -56,23 +51,5 @@ protected override DiagnosticInfo GetInstanceWithSeverityCore(DiagnosticSeverity
{
return new SyntaxDiagnosticInfo(this, severity);
}

#region Serialization

protected override void WriteTo(ObjectWriter writer)
{
base.WriteTo(writer);
writer.WriteInt32(this.Offset);
writer.WriteInt32(this.Width);
}

protected SyntaxDiagnosticInfo(ObjectReader reader)
: base(reader)
{
this.Offset = reader.ReadInt32();
this.Width = reader.ReadInt32();
}

#endregion
}
}
21 changes: 0 additions & 21 deletions src/Compilers/CSharp/Portable/Errors/XmlSyntaxDiagnosticInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@ namespace Microsoft.CodeAnalysis.CSharp
{
internal sealed class XmlSyntaxDiagnosticInfo : SyntaxDiagnosticInfo
{
static XmlSyntaxDiagnosticInfo()
{
ObjectBinder.RegisterTypeReader(typeof(XmlSyntaxDiagnosticInfo), r => new XmlSyntaxDiagnosticInfo(r));
}

private readonly XmlParseErrorCode _xmlErrorCode;

internal XmlSyntaxDiagnosticInfo(XmlParseErrorCode code, params object[] args)
Expand All @@ -38,22 +33,6 @@ protected override DiagnosticInfo GetInstanceWithSeverityCore(DiagnosticSeverity
return new XmlSyntaxDiagnosticInfo(this, severity);
}

#region Serialization

protected override void WriteTo(ObjectWriter writer)
{
base.WriteTo(writer);
writer.WriteUInt32((uint)_xmlErrorCode);
}

private XmlSyntaxDiagnosticInfo(ObjectReader reader)
: base(reader)
{
_xmlErrorCode = (XmlParseErrorCode)reader.ReadUInt32();
}

#endregion

public override string GetMessage(IFormatProvider? formatProvider = null)
{
var culture = formatProvider as CultureInfo;
Copy link
Member Author

Choose a reason for hiding this comment

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

writing this comment here above the generated file. THe generated file is where the buld of the deletion comes from as we no longer need to emit all that goop for syntax serialization.

Expand Down
Loading
Loading