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

Fixed VS crash during implicit conversion of null object in nullable walker #45974

Merged
merged 2 commits into from
Jul 16, 2020

Conversation

kevinsun-dev
Copy link
Contributor

@kevinsun-dev kevinsun-dev commented Jul 14, 2020

Fixed VS crash during implicit conversion due to null object. Documented in issue #45862.

Changes:

  • Added null check in Nullable Walker
  • Added test

@kevinsun-dev kevinsun-dev added this to the 16.8 milestone Jul 15, 2020
@kevinsun-dev kevinsun-dev self-assigned this Jul 15, 2020
@kevinsun-dev kevinsun-dev marked this pull request as ready for review July 15, 2020 14:07
@kevinsun-dev kevinsun-dev requested a review from a team as a code owner July 15, 2020 14:07
@gafter
Copy link
Member

gafter commented Jul 15, 2020

Can you please use more descriptive titles for PRs in the future?

Copy link
Member

@gafter gafter left a comment

Choose a reason for hiding this comment

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

😷

@kevinsun-dev kevinsun-dev changed the title Fixed Issue #45862 Fixed VS crash during implicit conversion of null object in nullable walker Jul 15, 2020
@kevinsun-dev
Copy link
Contributor Author

Can you please use more descriptive titles for PRs in the future?

Apologies, forgot to change the title from when it was a draft.

@@ -5537,7 +5537,7 @@ private Conversion GenerateConversionForConditionalOperator(BoundExpression sour
private static Conversion GenerateConversion(Conversions conversions, BoundExpression sourceExpression, TypeSymbol sourceType, TypeSymbol destinationType, bool fromExplicitCast, bool extensionMethodThisArgument)
{
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
bool useExpression = UseExpressionForConversion(sourceExpression);
bool useExpression = (sourceType is null) ? true : UseExpressionForConversion(sourceExpression);
Copy link
Member

Choose a reason for hiding this comment

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

Consider writing as sourceType is null || UseExpressionForConversion(sourceExpression).

};
}
}";
var comp = CreateCompilation(new[] { source }, parseOptions: TestOptions.Regular8, options: WithNonNullTypesTrue(TestOptions.ReleaseModule));
Copy link
Member

Choose a reason for hiding this comment

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

Can be simplified to var comp = CreateCompilation(source);

@@ -4780,6 +4780,37 @@ void F(System.Action<System.Object?, System.Action<System.Object!, System.Object
});
}

[Fact]
[WorkItem(45862, "https://github.com/dotnet/roslyn/issues/45862")]
public void Issue_45862()
Copy link
Member

Choose a reason for hiding this comment

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

Consider moving to NullableReferenceTypesTests.cs.

Copy link
Member

Choose a reason for hiding this comment

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

Consider giving the test a descriptive name, perhaps "SwitchExpressionNoType".


In reply to: 455193504 [](ancestors = 455193504)

@kevinsun-dev kevinsun-dev merged commit 37abccb into dotnet:master Jul 16, 2020
@ghost ghost modified the milestones: 16.8, Next Jul 16, 2020
@kevinsun-dev kevinsun-dev deleted the kevin-issue-45862 branch July 16, 2020 20:41
333fred added a commit to 333fred/roslyn that referenced this pull request Jul 17, 2020
* upstream/master: (86 commits)
  Better client / server logging (dotnet#46079)
  Fix OptProf config
  Update src/VisualStudio/LiveShare/Impl/AbstractGoToDefinitionWithFindUsagesServiceHandler.cs
  Do not execute RemoveUnnecessaryInlineSuppressions on generated code
  Update dependencies from https://github.com/dotnet/arcade build 20200715.6 (dotnet#46086)
  Use ISpanMapper before sending cross file results to LSP
  Add additional module initializers tests from review (dotnet#46020)
  Fix type in OptProf configuration
  bump to 500
  Fixed VS crash during implicit conversion of null object in nullable walker (dotnet#45974)
  Rename variable
  Fix KeyNotFound exception in RemoveUnnecessaryInlineSuppressionsDiagnosticAnalyzer
  Check modifiers on record positional members (dotnet#45898)
  Fix typos and link in Source Generators cookbook (dotnet#44372)
  Remove reference to non-existing VisualStudioInteractiveComponents.vsix from deployment VSIX. (dotnet#45979)
  Address feedback + fix tests
  Update dependencies from https://github.com/dotnet/roslyn build 20200711.1 (dotnet#45932)
  Make MetadataTypeName non-copyable
  Fix usage of GetService extension
  Consolidate service provider extensions
  ...
@JoeRobich JoeRobich modified the milestones: Next, 16.8.P1 Jul 20, 2020
333fred added a commit to 333fred/roslyn that referenced this pull request Jul 23, 2020
…to function-pointer-type-lookup

* upstream/features/function-pointers: (86 commits)
  Better client / server logging (dotnet#46079)
  Fix OptProf config
  Update src/VisualStudio/LiveShare/Impl/AbstractGoToDefinitionWithFindUsagesServiceHandler.cs
  Do not execute RemoveUnnecessaryInlineSuppressions on generated code
  Update dependencies from https://github.com/dotnet/arcade build 20200715.6 (dotnet#46086)
  Use ISpanMapper before sending cross file results to LSP
  Add additional module initializers tests from review (dotnet#46020)
  Fix type in OptProf configuration
  bump to 500
  Fixed VS crash during implicit conversion of null object in nullable walker (dotnet#45974)
  Rename variable
  Fix KeyNotFound exception in RemoveUnnecessaryInlineSuppressionsDiagnosticAnalyzer
  Check modifiers on record positional members (dotnet#45898)
  Fix typos and link in Source Generators cookbook (dotnet#44372)
  Remove reference to non-existing VisualStudioInteractiveComponents.vsix from deployment VSIX. (dotnet#45979)
  Address feedback + fix tests
  Update dependencies from https://github.com/dotnet/roslyn build 20200711.1 (dotnet#45932)
  Make MetadataTypeName non-copyable
  Fix usage of GetService extension
  Consolidate service provider extensions
  ...
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.

4 participants