Skip to content

Ensure we have stack spilling support for the recently-added expression node BoundReadOnlySpanFromArray#37057

Merged
gafter merged 1 commit into
dotnet:masterfrom
gafter:master-36856
Jul 9, 2019
Merged

Ensure we have stack spilling support for the recently-added expression node BoundReadOnlySpanFromArray#37057
gafter merged 1 commit into
dotnet:masterfrom
gafter:master-36856

Conversation

@gafter

@gafter gafter commented Jul 8, 2019

Copy link
Copy Markdown
Member

Fixes #36856

@dotnet/roslyn-compiler May I please have a couple of reviews of this tiny bug fix?

@gafter gafter added this to the 16.3 milestone Jul 8, 2019
@gafter gafter requested review from a team and jcouv July 8, 2019 22:48
@gafter gafter self-assigned this Jul 8, 2019
return UpdateExpression(builder, node.Update(node.OperatorKind, operand, node.ConstantValueOpt, node.MethodOpt, node.ResultKind, node.Type));
}

public override BoundNode VisitReadOnlySpanFromArray(BoundReadOnlySpanFromArray node)

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.

VisitReadOnlySpanFromArray [](start = 34, length = 26)

not related to this PR: I wonder if there is a systematic way we can catch those situations (all the walkers/rewriters to consider when adding new bound nodes)... Maybe an analyzer could force us to write those methods, if only to call the base?

@jcouv jcouv left a comment

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.

LGTM Thanks (iteration 1)

@jcouv jcouv self-assigned this Jul 9, 2019
}

[Fact, WorkItem(36856, "https://github.com/dotnet/roslyn/issues/36856")]
public void Crash36856()

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.

Crash36856 [](start = 20, length = 10)

Consider using a name that is actually descriptive, without having to visit the linked bug.

@333fred 333fred left a comment

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.

LGTM other than test name comment.

@gafter gafter merged commit ecb34ad into dotnet:master Jul 9, 2019
canton7 added a commit to canton7/roslyn that referenced this pull request Jul 9, 2019
…ncat-order

* upstream/master: (1532 commits)
  Ensure we have stack spilling support for the recently-added expression node `BoundReadOnlySpanFromArray` (dotnet#37057)
  Review feedback
  Avoid generating or emitting NullablePublicOnlyAttribute when no other nullable attributes are emitted (dotnet#37019)
  Respond to more feedback
  Fixes from feedback
  Call `.WithoutNullability()` less
  Fix ngen for assets from nuget
  Make NullableWalker aware of calls to Interlocked.CompareExchange
  fix error
  Update AnonymousObjectCreation implementation to be more robust to errors by using MemberIndexOpt. Address minor PR comments.
  Add support to ngen assets included from nuget package
  Modified node removal to keep original leading trivia
  Fix Solution.WithDocumentFilePath not updating the file path of the tree
  Improve docs.
  PR Feedback cleanup.
  Use pattern-matching in MetadataWriter for readability and possibly performance. (dotnet#36219)
  Renamed helpers in SyntaxFactsService.
  More RefactoringHelpers tests (mostly for extraction).
  Add general tests for RefactoringHelpersService.
  Optimize flow analysis assembly
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

csc.exe crash when I use System.Text.Json.Serialization

4 participants