[Xamlc] remove unused locals #620

Merged
merged 1 commit into from Dec 13, 2016

Conversation

Projects
None yet
3 participants
@StephaneDelcroix
Member

StephaneDelcroix commented Dec 7, 2016

Description of Change

Remove unused local variables

This removes 2 variables and 4 instructions (4 to 12B) each time we use a ServiceProvider.
e.g., in StylesTests, it saves 14 variables and 72B of code.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
@samhouts

Seems like there should be a unit test for the unused locals. I'd like to see an example of when this would happen.

+ continue;
+
+ //find the Stloc instruction
+ var instruction = (from instr in self.Instructions where instr.OpCode.Code == Code.Stloc && instr.Operand == varDef select instr).First();

This comment has been minimized.

@samhouts

samhouts Dec 12, 2016

Member

Why not self.Instructions.First(instr => instr.OpCode.Code == Code.Stloc && instr.Operand == varDef)?

@samhouts

samhouts Dec 12, 2016

Member

Why not self.Instructions.First(instr => instr.OpCode.Code == Code.Stloc && instr.Operand == varDef)?

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Dec 12, 2016

Member

this comes from a refactoring. no good reason to have it in one way or another

@StephaneDelcroix

StephaneDelcroix Dec 12, 2016

Member

this comes from a refactoring. no good reason to have it in one way or another

@StephaneDelcroix

Well, this won't happen often, it requires this kind of code

public void M() {
        var a = 1;
    }

generating

.method public hidebysig 
        instance void M () cil managed 
    {
        // Method begins at RVA 0x2050
        // Code size 4 (0x4)
        .maxstack 1
        .locals init (
            [0] int32
        )

        IL_0000: nop                  // Do nothing (No operation)
        IL_0001: ldc.i4.1             // Push 1 onto the stack as int32
        IL_0002: stloc.0              // Pop a value from stack into local variable 0
        IL_0003: ret                  // Return from method, possibly with a value
    } 

This doesn't happen often, and NEVER in code generated by XamlC. BUT you approved another PR that replaces sdloc/ldloc by dup/stloc and in that case we end up with unused variables.

+ continue;
+
+ //find the Stloc instruction
+ var instruction = (from instr in self.Instructions where instr.OpCode.Code == Code.Stloc && instr.Operand == varDef select instr).First();

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Dec 12, 2016

Member

this comes from a refactoring. no good reason to have it in one way or another

@StephaneDelcroix

StephaneDelcroix Dec 12, 2016

Member

this comes from a refactoring. no good reason to have it in one way or another

@samhouts

Ahhh, thank you for the additional context, @StephaneDelcroix. I couldn't figure out how XamlC was going to be afflicted with unused locals, but your explanation makes complete sense.

@StephaneDelcroix StephaneDelcroix merged commit 4625f69 into master Dec 13, 2016

2 of 6 checks passed

Android-UITests-C8 Started TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1
Details
iOS10-UITests-C8 Started TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10
Details
iOS8-UITests-C8 Started TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8
Details
iOS9-UITests-C8 Started TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: Windows Debug : Tests passed: 3685, ignored: 10
Details

@StephaneDelcroix StephaneDelcroix deleted the xamlc_removeUnusedLocals branch Dec 13, 2016

@samhouts samhouts added this to the 2.3.4 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment