Simplifier.ReduceAsync fails detect type-inference change when removing necessary cast

Ported from TFS WorkItem: 742781

Repro Steps:

Run the code below, note the lack of explicit type annotations on the variable declarations to ensure correct type inference. I'm trying to use the Expand/Reduce API to detect and correct for potential breaking changes on project upgrade.

Code

Imports System.Threading.Tasks
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.VisualBasic
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
Imports Microsoft.CodeAnalysis.VisualBasic.Semantics
Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
Imports System.Console

Module Module1

    Sub Main()
        System.Windows.Forms.Application.Run(New AsyncConsoleApplicationContext(AddressOf MainAsync))
    End Sub

    Async Function MainAsync() As Task
        Dim source =
        <source>
Imports System.Collections
Imports System.Console

Module M
    Sub Main()

        Dim i = 1
        Dim arr() = {1, 2, 3}

    End Sub
End Module
</source>

        Dim sourceText = Text.SourceText.From(source)

        Dim tnv = TextAndVersion.Create(sourceText, VersionStamp.Default)

        Dim ws As New CustomWorkspace()

        Dim pId = ProjectId.CreateNewId()

        ws.AddProject(ProjectInfo.Create(pId, VersionStamp.Default, "Test", "Test", LanguageNames.VisualBasic))

        Dim dId = DocumentId.CreateNewId(pId)

        ws.AddDocument(DocumentInfo.Create(dId, "M.vb", loader:=TextLoader.From(tnv), isGenerated:=True))

        Dim dotNet40References = {New MetadataFileReference("c:\windows\microsoft.net\framework\v4.0.30319\mscorlib.dll"),
                                  New MetadataFileReference("c:\windows\microsoft.net\framework\v4.0.30319\microsoft.visualbasic.dll")}

        Dim solution2 = ws.CurrentSolution.WithProjectMetadataReferences(pId, dotNet40References)

        Dim optionInferOn As CompilationOptions = (Await solution2.GetProjectAsync(pId)).CompilationOptions
        Dim optionInferOff = optionInferOn.WithOptionInfer(False)

        Dim document2 = solution2.GetDocument(dId)

        Dim root2 As CompilationUnitSyntax = Await document2.GetSyntaxRootAsync()

        WriteLine("=== Option Infer On === ")
        WriteLine(root2.ToFullString())
        WriteLine()

        Dim expandedRoot = Await ExpandAsync(document2)

        WriteLine("Expanded:")
        WriteLine(expandedRoot.ToFullString())
        WriteLine()

        WriteLine("Converting...")
        WriteLine()

        Dim solution4 = solution2.WithDocumentSyntaxRoot(dId, expandedRoot).WithProjectCompilationOptions(pId, optionInferOff)

        Dim document4 = Await Formatting.Formatter.FormatAsync(Await Simplification.Simplifier.ReduceAsync(solution4.GetDocument(dId)))

        Dim root4 As CompilationUnitSyntax = Await document4.GetSyntaxRootAsync()
        WriteLine()

        WriteLine("=== Option Infer Off ===")
        WriteLine(root4.ToFullString())
        WriteLine()

        WriteLine("Expanded:")
        WriteLine((Await ExpandAsync(document4)).ToFullString())
        WriteLine()
    End Function

    Async Function ExpandAsync(document As Document) As Task(Of CompilationUnitSyntax)

        Dim root = Await document.GetSyntaxRootAsync()

        Dim replacements As New Dictionary(Of ExecutableStatementSyntax, ExecutableStatementSyntax)

        For Each statement In root.DescendantNodes.OfType(Of ExecutableStatementSyntax)()
            replacements.Add(statement, Await Simplification.Simplifier.ExpandAsync(statement, document))
        Next

        Return root.ReplaceNodes(replacements.Keys, Function(old, rewritten) replacements(old))

    End Function

End Module

Class AsyncConsoleApplicationContext
    Inherits System.Windows.Forms.ApplicationContext

    Sub New(mainMethodAsync As Func(Of Task))

        ExecuteMainAsync(mainMethodAsync)

    End Sub

    Async Sub ExecuteMainAsync(mainMethod As Func(Of Task))
        Await mainMethod()

        ExitThread()
        End
    End Sub

End Class

Revisions:

Created By Anthony D. Green (7/19/2013 7:03:35 PM)

Edited By David Sterling (VS) (7/23/2013 10:01:03 AM)

I think we're missing where the bug is here. Can you point it out?

Edited By Anthony D. Green (7/25/2013 4:15:11 PM)

The code is expanded under Option Infer On where the type of i will be Integer and arr will be Integer() and then reduced under Option Infer Off where the type of i will be Object and arr will be Object() and the array literal will have Object() type and each of its expressions will be implicitly boxed. To correctly simplify this the explicit type annotations must be inserted - Dim i As Integer, Dim arr() As Integer (or Dim arr As Integer()).

Expand/Reduce needs to add explicit types to type inferred locals and try removing them to see if the type changes.

Edited By Alex Turner (7/29/2013 9:53:08 AM)

Let's change the default behavior of reducing to preserve the original explicitness of a declaration - if it was using var or an inferred Dim, let's keep it that way, and if it was using an explicit type, let's keep it that way. This would be a third state of Reduce option that controls the explicitness of declarations.

Once we've introduced this default behavior, let's make the Reduce option to control the behavior internal, as this behavior is likely what almost all external refactorings will want.

Updated repro code that accounts for Roslyn API changes since this issue was logged:

Imports System.Threading.Tasks
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.VisualBasic
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
Imports System.Console

Module Module1

    Sub Main()
        System.Windows.Forms.Application.Run(New AsyncConsoleApplicationContext(AddressOf MainAsync))
    End Sub

    Async Function MainAsync() As Task
        Dim source =
        <source>
Imports System.Collections
Imports System.Console

Module M
    Sub Main()

        Dim i = 1       
        Dim arr() = {1, 2, 3}

    End Sub
End Module
</source>

        Dim sourceText = Text.SourceText.From(source)

        Dim tnv = TextAndVersion.Create(sourceText, VersionStamp.Default)

        Dim ws As New AdhocWorkspace()

        Dim pId = ProjectId.CreateNewId()

        ws.AddProject(ProjectInfo.Create(pId, VersionStamp.Default, "Test", "Test", LanguageNames.VisualBasic))

        Dim dId = DocumentId.CreateNewId(pId)

        ws.AddDocument(DocumentInfo.Create(dId, "M.vb", loader:=TextLoader.From(tnv), isGenerated:=True))

        Dim dotNet40References = {MetadataReference.CreateFromFile("c:\windows\microsoft.net\framework\v4.0.30319\mscorlib.dll"),
                                  MetadataReference.CreateFromFile("c:\windows\microsoft.net\framework\v4.0.30319\microsoft.visualbasic.dll")}

        Dim solution2 = ws.CurrentSolution.WithProjectMetadataReferences(pId, dotNet40References)

        Dim optionInferOn As VisualBasicCompilationOptions = solution2.GetProject(pId).CompilationOptions
        Dim optionInferOff = optionInferOn.WithOptionInfer(False)

        Dim document2 = solution2.GetDocument(dId)

        Dim root2 As CompilationUnitSyntax = Await document2.GetSyntaxRootAsync()

        WriteLine("=== Option Infer On === ")
        WriteLine(root2.ToFullString())
        WriteLine()

        Dim expandedRoot = Await ExpandAsync(document2)

        WriteLine("Expanded:")
        WriteLine(expandedRoot.ToFullString())
        WriteLine()

        WriteLine("Converting...")
        WriteLine()

        Dim solution4 = solution2.WithDocumentSyntaxRoot(dId, expandedRoot).WithProjectCompilationOptions(pId, optionInferOff)

        Dim document4 = Await Formatting.Formatter.FormatAsync(Await Simplification.Simplifier.ReduceAsync(solution4.GetDocument(dId)))

        Dim root4 As CompilationUnitSyntax = Await document4.GetSyntaxRootAsync()
        WriteLine()

        WriteLine("=== Option Infer Off ===")
        WriteLine(root4.ToFullString())
        WriteLine()

        WriteLine("Expanded:")
        WriteLine((Await ExpandAsync(document4)).ToFullString())
        WriteLine()
    End Function

    Async Function ExpandAsync(document As Document) As Task(Of CompilationUnitSyntax)

        Dim root = Await document.GetSyntaxRootAsync()

        Dim replacements As New Dictionary(Of ExecutableStatementSyntax, ExecutableStatementSyntax)

        For Each statement In root.DescendantNodes.OfType(Of ExecutableStatementSyntax)()
            replacements.Add(statement, Await Simplification.Simplifier.ExpandAsync(statement, document))
        Next

        Return root.ReplaceNodes(replacements.Keys, Function(old, rewritten) replacements(old))

    End Function

End Module

Class AsyncConsoleApplicationContext
    Inherits System.Windows.Forms.ApplicationContext

    Sub New(mainMethodAsync As Func(Of Task))

        ExecuteMainAsync(mainMethodAsync)

    End Sub

    Async Sub ExecuteMainAsync(mainMethod As Func(Of Task))
        Await mainMethod()

        ExitThread()
        End
    End Sub

End Class

If I understand correctly, this issue is requesting that when we expand, we should always change all inferred variables to instead specify their types explicitly. The simplifier should then remove any explicit typing that was inserted by the expander if Option Infer is On or preserve such typing if Option Infer is Off.

This seems like a new feature request for the expand / reduce APIs rather than a simple bug fix and I don't think we should change this now.

Things to consider when we get to this -

For completeness, we should also implement this behavior in C#
VB supports inferring anonymous delegate types for lambdas, we will need to figure out what type to give such for type inferred variables initialized with lambdas when we expand. Anonymous delegate type can't be specified explicitly - so we will either have to expand to corresponding Func / Action types that can be specified (which would probably not change semantics - but would change the type) or set them to Object (which will probably cause some code to end up running late bound). Similarly for anonymous types.
We need to be careful to not touch explicit typing that the user already had in their code (e.g. reducer should never remove types that the user had explicitly specified in their code even before expansion).
We should also expand / reduce types for lambda arguments that were inferred based on type specified on left hand side e.g. for Dim x As Func(Of Integer, Integer) = Function(x) x, we should expand to Dim x As Func(Of Integer, Integer) = Function(x As Integer) x. Similarly in cases where type is inferred based on generic type arguments e.g. when Foo(Of T)(x As Func(Of T, T)) is invoked as Foo(Of Integer)(Function(x) x), we should expand to Foo(Of Integer)(Function(x As Integer) x.
If we wanted to make all inferred types explicit during expansion then we should also consider inserting explicit type arguments for inferred generic methods e.g. when Foo(Of T)(x As T) is invoked as Foo(1), we should expand to Foo(Of Integer)(1).

Closing out as unplanned.

Merge pull request #476 from sharwell/refactorings

Navigation Menu

Repository navigation

Simplifier.ReduceAsync fails detect type-inference change when removing necessary cast #476

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Simplifier.ReduceAsync fails detect type-inference change when removing necessary cast #476

Description

Activity

shyamnamboodiripad commented on Apr 16, 2015

shyamnamboodiripad commented on Apr 17, 2015

CyrusNajmabadi commented on Nov 29, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions