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

VB -> C#: problems with pulling variables out of scope #913

Closed
KunzeAndreas opened this issue Jul 3, 2022 · 5 comments
Closed

VB -> C#: problems with pulling variables out of scope #913

KunzeAndreas opened this issue Jul 3, 2022 · 5 comments
Labels
output logic error A bug where the converted output behaves differently to the input code regression Something that used to work VB -> C# Specific to VB -> C# conversion

Comments

@KunzeAndreas
Copy link

VB.Net input code

Private Shared Sub ProblemsWithPullingVariablesOut()
      ' example 1
      For Each a In New List(Of String)
          Dim b As Long
          If a = "" Then
              b = 1
          End If
          DoSomeImportantStuff()
      Next

      ' example 2
      Dim c As String
      Do While True
          Dim d As Long
          If c = "" Then
              d = 1
          End If

         DoSomeImportantStuff()
         Exit Do
     Loop
 End Sub
 Private Shared Sub DoSomeImportantStuff()
     Debug.Print("very important")
 End Sub

Erroneous output

 // example 1
        private static void ProblemsWithPullingVariablesOut()
        {
            ;

            // example 2
            var c = default(string);
            ;
        }
        private static void DoSomeImportantStuff()
        {
            Debug.Print("very important");
        }

Details

  • Product in use: VS extension
  • Version in use: 9.0.1.0
  • Did you see it working in a previous version, which? yes (8.5)
@KunzeAndreas KunzeAndreas added the VB -> C# Specific to VB -> C# conversion label Jul 3, 2022
@GrahamTheCoder
Copy link
Member

Oh dear, that looks pretty broken 😲

@GrahamTheCoder GrahamTheCoder added regression Something that used to work output logic error A bug where the converted output behaves differently to the input code labels Jul 3, 2022
@GrahamTheCoder
Copy link
Member

I think I can see the problem - it has a null initializer. I'll try to put in a quick fix now

@Yozer
Copy link
Member

Yozer commented Jul 3, 2022

@GrahamTheCoder I guess we're doing some kind of data flow analysis to see if local variables are always assigned before read.
I wonder if the logic "pull variables before the loop" can be skipped when the initializer in C# is null.
In these examples, I think it's safe to declare b and d inside a loop.

@GrahamTheCoder
Copy link
Member

I like the general idea of not pulling out variables when it isn't needed. But the mechanism - making decisions based on the C# syntax tree - is usually a bad idea. You can never know exactly why it ended up that way. It may well be due to sensible flow analysis, or it could just be a style preference. I don't know in this case if any analysis takes into account this possibility properly. I'd rather re-do the analysis (even calling the same method) at the point we expect something particular of it for clarity if needed and not in the same method.

@KunzeAndreas
Copy link
Author

Thank you for the quick fix, I will try this in the next few days on our codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
output logic error A bug where the converted output behaves differently to the input code regression Something that used to work VB -> C# Specific to VB -> C# conversion
Projects
None yet
Development

No branches or pull requests

3 participants